diff options
Diffstat (limited to 'spec')
147 files changed, 3957 insertions, 1039 deletions
diff --git a/spec/controllers/namespaces_controller_spec.rb b/spec/controllers/namespaces_controller_spec.rb deleted file mode 100644 index 2b334ed1172..00000000000 --- a/spec/controllers/namespaces_controller_spec.rb +++ /dev/null @@ -1,118 +0,0 @@ -require 'spec_helper' - -describe NamespacesController do - let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } - - describe "GET show" do - context "when the namespace belongs to a user" do - let!(:other_user) { create(:user) } - - it "redirects to the user's page" do - get :show, id: other_user.username - - expect(response).to redirect_to(user_path(other_user)) - end - end - - context "when the namespace belongs to a group" do - let!(:group) { create(:group) } - - context "when the group is public" do - context "when not signed in" do - it "redirects to the group's page" do - get :show, id: group.path - - expect(response).to redirect_to(group_path(group)) - end - end - - context "when signed in" do - before do - sign_in(user) - end - - it "redirects to the group's page" do - get :show, id: group.path - - expect(response).to redirect_to(group_path(group)) - end - end - end - - context "when the group is private" do - before do - group.update_attribute(:visibility_level, Group::PRIVATE) - end - - context "when not signed in" do - it "redirects to the sign in page" do - get :show, id: group.path - expect(response).to redirect_to(new_user_session_path) - end - end - - context "when signed in" do - before do - sign_in(user) - end - - context "when the user has access to the group" do - before do - group.add_developer(user) - end - - context "when the user is blocked" do - before do - user.block - end - - it "redirects to the sign in page" do - get :show, id: group.path - - expect(response).to redirect_to(new_user_session_path) - end - end - - context "when the user isn't blocked" do - it "redirects to the group's page" do - get :show, id: group.path - - expect(response).to redirect_to(group_path(group)) - end - end - end - - context "when the user doesn't have access to the group" do - it "responds with status 404" do - get :show, id: group.path - - expect(response).to have_http_status(404) - end - end - end - end - end - - context "when the namespace doesn't exist" do - context "when signed in" do - before do - sign_in(user) - end - - it "responds with status 404" do - get :show, id: "doesntexist" - - expect(response).to have_http_status(404) - end - end - - context "when not signed in" do - it "redirects to the sign in page" do - get :show, id: "doesntexist" - - expect(response).to redirect_to(new_user_session_path) - end - end - end - end -end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 9444a50b1ce..52d13fb6f9e 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -5,7 +5,6 @@ describe Projects::BlobController do let(:user) { create(:user) } before do - user = create(:user) project.team << [user, :master] sign_in(user) diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb index 2896636db5a..da59642f24d 100644 --- a/spec/controllers/projects/boards/issues_controller_spec.rb +++ b/spec/controllers/projects/boards/issues_controller_spec.rb @@ -1,17 +1,20 @@ require 'spec_helper' describe Projects::Boards::IssuesController do - let(:project) { create(:project_with_board) } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } + let(:guest) { create(:user) } let(:planning) { create(:label, project: project, name: 'Planning') } let(:development) { create(:label, project: project, name: 'Development') } - let!(:list1) { create(:list, board: project.board, label: planning, position: 0) } - let!(:list2) { create(:list, board: project.board, label: development, position: 1) } + let!(:list1) { create(:list, board: board, label: planning, position: 0) } + let!(:list2) { create(:list, board: board, label: development, position: 1) } before do project.team << [user, :master] + project.team << [guest, :guest] end describe 'GET index' do @@ -22,7 +25,7 @@ describe Projects::Boards::IssuesController do create(:labeled_issue, project: project, labels: [development]) create(:labeled_issue, project: project, labels: [development], assignee: johndoe) - list_issues user: user, list_id: list2 + list_issues user: user, board: board, list: list2 parsed_response = JSON.parse(response.body) @@ -31,9 +34,17 @@ describe Projects::Boards::IssuesController do end end + context 'with invalid board id' do + it 'returns a not found 404 response' do + list_issues user: user, board: 999, list: list2 + + expect(response).to have_http_status(404) + end + end + context 'with invalid list id' do it 'returns a not found 404 response' do - list_issues user: user, list_id: 999 + list_issues user: user, board: board, list: 999 expect(response).to have_http_status(404) end @@ -45,19 +56,75 @@ describe Projects::Boards::IssuesController do allow(Ability).to receive(:allowed?).with(user, :read_issue, project).and_return(false) end - it 'returns a successful 403 response' do - list_issues user: user, list_id: list2 + it 'returns a forbidden 403 response' do + list_issues user: user, board: board, list: list2 expect(response).to have_http_status(403) end end - def list_issues(user:, list_id:) + def list_issues(user:, board:, list:) sign_in(user) get :index, namespace_id: project.namespace.to_param, project_id: project.to_param, - list_id: list_id.to_param + board_id: board.to_param, + list_id: list.to_param + end + end + + describe 'POST create' do + context 'with valid params' do + it 'returns a successful 200 response' do + create_issue user: user, board: board, list: list1, title: 'New issue' + + expect(response).to have_http_status(200) + end + + it 'returns the created issue' do + create_issue user: user, board: board, list: list1, title: 'New issue' + + expect(response).to match_response_schema('issue') + end + end + + context 'with invalid params' do + context 'when title is nil' do + it 'returns an unprocessable entity 422 response' do + create_issue user: user, board: board, list: list1, title: nil + + expect(response).to have_http_status(422) + end + end + + context 'when list does not belongs to project board' do + it 'returns a not found 404 response' do + list = create(:list) + + create_issue user: user, board: board, list: list, title: 'New issue' + + expect(response).to have_http_status(404) + end + end + end + + context 'with unauthorized user' do + it 'returns a forbidden 403 response' do + create_issue user: guest, board: board, list: list1, title: 'New issue' + + expect(response).to have_http_status(403) + end + end + + def create_issue(user:, board:, list:, title:) + sign_in(user) + + post :create, namespace_id: project.namespace.to_param, + project_id: project.to_param, + board_id: board.to_param, + list_id: list.to_param, + issue: { title: title }, + format: :json end end @@ -66,13 +133,13 @@ describe Projects::Boards::IssuesController do context 'with valid params' do it 'returns a successful 200 response' do - move user: user, issue: issue, from_list_id: list1.id, to_list_id: list2.id + move user: user, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_http_status(200) end it 'moves issue to the desired list' do - move user: user, issue: issue, from_list_id: list1.id, to_list_id: list2.id + move user: user, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(issue.reload.labels).to contain_exactly(development) end @@ -80,13 +147,19 @@ describe Projects::Boards::IssuesController do context 'with invalid params' do it 'returns a unprocessable entity 422 response for invalid lists' do - move user: user, issue: issue, from_list_id: nil, to_list_id: nil + move user: user, board: board, issue: issue, from_list_id: nil, to_list_id: nil expect(response).to have_http_status(422) end + it 'returns a not found 404 response for invalid board id' do + move user: user, board: 999, issue: issue, from_list_id: list1.id, to_list_id: list2.id + + expect(response).to have_http_status(404) + end + it 'returns a not found 404 response for invalid issue id' do - move user: user, issue: 999, from_list_id: list1.id, to_list_id: list2.id + move user: user, board: board, issue: 999, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_http_status(404) end @@ -99,18 +172,19 @@ describe Projects::Boards::IssuesController do project.team << [guest, :guest] end - it 'returns a successful 403 response' do - move user: guest, issue: issue, from_list_id: list1.id, to_list_id: list2.id + it 'returns a forbidden 403 response' do + move user: guest, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_http_status(403) end end - def move(user:, issue:, from_list_id:, to_list_id:) + def move(user:, board:, issue:, from_list_id:, to_list_id:) sign_in(user) patch :update, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, id: issue.to_param, from_list_id: from_list_id, to_list_id: to_list_id, diff --git a/spec/controllers/projects/boards/lists_controller_spec.rb b/spec/controllers/projects/boards/lists_controller_spec.rb index 709006a3601..34d6119429d 100644 --- a/spec/controllers/projects/boards/lists_controller_spec.rb +++ b/spec/controllers/projects/boards/lists_controller_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe Projects::Boards::ListsController do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let(:guest) { create(:user) } @@ -13,7 +13,7 @@ describe Projects::Boards::ListsController do describe 'GET index' do it 'returns a successful 200 response' do - read_board_list user: user + read_board_list user: user, board: board expect(response).to have_http_status(200) expect(response.content_type).to eq 'application/json' @@ -22,7 +22,7 @@ describe Projects::Boards::ListsController do it 'returns a list of board lists' do create(:list, board: board) - read_board_list user: user + read_board_list user: user, board: board parsed_response = JSON.parse(response.body) @@ -37,17 +37,18 @@ describe Projects::Boards::ListsController do end it 'returns a forbidden 403 response' do - read_board_list user: user + read_board_list user: user, board: board expect(response).to have_http_status(403) end end - def read_board_list(user:) + def read_board_list(user:, board:) sign_in(user) get :index, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, format: :json end end @@ -57,13 +58,13 @@ describe Projects::Boards::ListsController do let(:label) { create(:label, project: project, name: 'Development') } it 'returns a successful 200 response' do - create_board_list user: user, label_id: label.id + create_board_list user: user, board: board, label_id: label.id expect(response).to have_http_status(200) end it 'returns the created list' do - create_board_list user: user, label_id: label.id + create_board_list user: user, board: board, label_id: label.id expect(response).to match_response_schema('list') end @@ -72,7 +73,7 @@ describe Projects::Boards::ListsController do context 'with invalid params' do context 'when label is nil' do it 'returns a not found 404 response' do - create_board_list user: user, label_id: nil + create_board_list user: user, board: board, label_id: nil expect(response).to have_http_status(404) end @@ -82,7 +83,7 @@ describe Projects::Boards::ListsController do it 'returns a not found 404 response' do label = create(:label, name: 'Development') - create_board_list user: user, label_id: label.id + create_board_list user: user, board: board, label_id: label.id expect(response).to have_http_status(404) end @@ -93,17 +94,18 @@ describe Projects::Boards::ListsController do it 'returns a forbidden 403 response' do label = create(:label, project: project, name: 'Development') - create_board_list user: guest, label_id: label.id + create_board_list user: guest, board: board, label_id: label.id expect(response).to have_http_status(403) end end - def create_board_list(user:, label_id:) + def create_board_list(user:, board:, label_id:) sign_in(user) post :create, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, list: { label_id: label_id }, format: :json end @@ -115,13 +117,13 @@ describe Projects::Boards::ListsController do context 'with valid position' do it 'returns a successful 200 response' do - move user: user, list: planning, position: 1 + move user: user, board: board, list: planning, position: 1 expect(response).to have_http_status(200) end it 'moves the list to the desired position' do - move user: user, list: planning, position: 1 + move user: user, board: board, list: planning, position: 1 expect(planning.reload.position).to eq 1 end @@ -129,7 +131,7 @@ describe Projects::Boards::ListsController do context 'with invalid position' do it 'returns an unprocessable entity 422 response' do - move user: user, list: planning, position: 6 + move user: user, board: board, list: planning, position: 6 expect(response).to have_http_status(422) end @@ -137,7 +139,7 @@ describe Projects::Boards::ListsController do context 'with invalid list id' do it 'returns a not found 404 response' do - move user: user, list: 999, position: 1 + move user: user, board: board, list: 999, position: 1 expect(response).to have_http_status(404) end @@ -145,17 +147,18 @@ describe Projects::Boards::ListsController do context 'with unauthorized user' do it 'returns a forbidden 403 response' do - move user: guest, list: planning, position: 6 + move user: guest, board: board, list: planning, position: 6 expect(response).to have_http_status(403) end end - def move(user:, list:, position:) + def move(user:, board:, list:, position:) sign_in(user) patch :update, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, id: list.to_param, list: { position: position }, format: :json @@ -167,19 +170,19 @@ describe Projects::Boards::ListsController do context 'with valid list id' do it 'returns a successful 200 response' do - remove_board_list user: user, list: planning + remove_board_list user: user, board: board, list: planning expect(response).to have_http_status(200) end it 'removes list from board' do - expect { remove_board_list user: user, list: planning }.to change(board.lists, :size).by(-1) + expect { remove_board_list user: user, board: board, list: planning }.to change(board.lists, :size).by(-1) end end context 'with invalid list id' do it 'returns a not found 404 response' do - remove_board_list user: user, list: 999 + remove_board_list user: user, board: board, list: 999 expect(response).to have_http_status(404) end @@ -187,17 +190,18 @@ describe Projects::Boards::ListsController do context 'with unauthorized user' do it 'returns a forbidden 403 response' do - remove_board_list user: guest, list: planning + remove_board_list user: guest, board: board, list: planning expect(response).to have_http_status(403) end end - def remove_board_list(user:, list:) + def remove_board_list(user:, board:, list:) sign_in(user) delete :destroy, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, id: list.to_param, format: :json end @@ -206,13 +210,13 @@ describe Projects::Boards::ListsController do describe 'POST generate' do context 'when board lists is empty' do it 'returns a successful 200 response' do - generate_default_board_lists user: user + generate_default_lists user: user, board: board expect(response).to have_http_status(200) end it 'returns the defaults lists' do - generate_default_board_lists user: user + generate_default_lists user: user, board: board expect(response).to match_response_schema('lists') end @@ -222,7 +226,7 @@ describe Projects::Boards::ListsController do it 'returns an unprocessable entity 422 response' do create(:list, board: board) - generate_default_board_lists user: user + generate_default_lists user: user, board: board expect(response).to have_http_status(422) end @@ -230,17 +234,18 @@ describe Projects::Boards::ListsController do context 'with unauthorized user' do it 'returns a forbidden 403 response' do - generate_default_board_lists user: guest + generate_default_lists user: guest, board: board expect(response).to have_http_status(403) end end - def generate_default_board_lists(user:) + def generate_default_lists(user:, board:) sign_in(user) post :generate, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, format: :json end end diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index 6f6e608e1f3..cc19035740e 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -9,16 +9,71 @@ describe Projects::BoardsController do sign_in(user) end + describe 'GET index' do + it 'creates a new project board when project does not have one' do + expect { list_boards }.to change(project.boards, :count).by(1) + end + + context 'when format is HTML' do + it 'renders template' do + list_boards + + expect(response).to render_template :index + expect(response.content_type).to eq 'text/html' + end + end + + context 'when format is JSON' do + it 'returns a list of project boards' do + create_list(:board, 2, project: project) + + list_boards format: :json + + parsed_response = JSON.parse(response.body) + + expect(response).to match_response_schema('boards') + expect(parsed_response.length).to eq 2 + end + end + + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) + end + + it 'returns a not found 404 response' do + list_boards + + expect(response).to have_http_status(404) + end + end + + def list_boards(format: :html) + get :index, namespace_id: project.namespace.to_param, + project_id: project.to_param, + format: format + end + end + describe 'GET show' do - it 'creates a new board when project does not have one' do - expect { read_board }.to change(Board, :count).by(1) + let!(:board) { create(:board, project: project) } + + context 'when format is HTML' do + it 'renders template' do + read_board board: board + + expect(response).to render_template :show + expect(response.content_type).to eq 'text/html' + end end - it 'renders HTML template' do - read_board + context 'when format is JSON' do + it 'returns project board' do + read_board board: board, format: :json - expect(response).to render_template :show - expect(response.content_type).to eq 'text/html' + expect(response).to match_response_schema('board') + end end context 'with unauthorized user' do @@ -27,16 +82,27 @@ describe Projects::BoardsController do allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) end - it 'returns a successful 404 response' do - read_board + it 'returns a not found 404 response' do + read_board board: board + + expect(response).to have_http_status(404) + end + end + + context 'when board does not belong to project' do + it 'returns a not found 404 response' do + another_board = create(:board) + + read_board board: another_board expect(response).to have_http_status(404) end end - def read_board(format: :html) + def read_board(board:, format: :html) get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, + id: board.to_param, format: format end end diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 7e440193d7b..646b097d74e 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -102,15 +102,16 @@ describe Projects::CommitController do describe "as patch" do include_examples "export as", :patch let(:format) { :patch } + let(:commit2) { project.commit('498214de67004b1da3d820901307bed2a68a8ef6') } it "is a git email patch" do - go(id: commit.id, format: format) + go(id: commit2.id, format: format) - expect(response.body).to start_with("From #{commit.id}") + expect(response.body).to start_with("From #{commit2.id}") end it "contains a git diff" do - go(id: commit.id, format: format) + go(id: commit2.id, format: format) expect(response.body).to match(/^diff --git/) end @@ -135,6 +136,8 @@ describe Projects::CommitController do describe "GET branches" do it "contains branch and tags information" do + commit = project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + get(:branches, namespace_id: project.namespace.to_param, project_id: project.to_param, @@ -254,16 +257,17 @@ describe Projects::CommitController do end let(:existing_path) { '.gitmodules' } + let(:commit2) { project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } context 'when the commit exists' do context 'when the user has access to the project' do context 'when the path exists in the diff' do it 'enables diff notes' do - diff_for_path(id: commit.id, old_path: existing_path, new_path: existing_path) + diff_for_path(id: commit2.id, old_path: existing_path, new_path: existing_path) expect(assigns(:diff_notes_disabled)).to be_falsey expect(assigns(:comments_target)).to eq(noteable_type: 'Commit', - commit_id: commit.id) + commit_id: commit2.id) end it 'only renders the diffs for the path given' do @@ -272,7 +276,7 @@ describe Projects::CommitController do meth.call(diffs) end - diff_for_path(id: commit.id, old_path: existing_path, new_path: existing_path) + diff_for_path(id: commit2.id, old_path: existing_path, new_path: existing_path) end end diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb index 2518a48e336..1ac7e03a2db 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -10,15 +10,38 @@ describe Projects::CommitsController do end describe "GET show" do - context "as atom feed" do - it "renders as atom" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: "master", - format: "atom") - expect(response).to be_success - expect(response.content_type).to eq('application/atom+xml') + context "when the ref name ends in .atom" do + render_views + + context "when the ref does not exist with the suffix" do + it "renders as atom" do + get(:show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: "master.atom") + + expect(response).to be_success + expect(response.content_type).to eq('application/atom+xml') + end + end + + context "when the ref exists with the suffix" do + before do + commit = project.repository.commit('master') + + allow_any_instance_of(Repository).to receive(:commit).and_call_original + allow_any_instance_of(Repository).to receive(:commit).with('master.atom').and_return(commit) + + get(:show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: "master.atom") + end + + it "renders as HTML" do + expect(response).to be_success + expect(response.content_type).to eq('text/html') + end end end end diff --git a/spec/controllers/projects/graphs_controller_spec.rb b/spec/controllers/projects/graphs_controller_spec.rb new file mode 100644 index 00000000000..74e6603b0cb --- /dev/null +++ b/spec/controllers/projects/graphs_controller_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe Projects::GraphsController do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + sign_in(user) + project.team << [user, :master] + end + + describe 'GET #languages' do + let(:linguist_repository) do + double(languages: { + 'Ruby' => 1000, + 'CoffeeScript' => 350, + 'PowerShell' => 15 + }) + end + + let(:expected_values) do + ps_color = "##{Digest::SHA256.hexdigest('PowerShell')[0...6]}" + [ + # colors from Linguist: + { label: "Ruby", color: "#701516", highlight: "#701516" }, + { label: "CoffeeScript", color: "#244776", highlight: "#244776" }, + # colors from SHA256 fallback: + { label: "PowerShell", color: ps_color, highlight: ps_color } + ] + end + + before do + allow(Linguist::Repository).to receive(:new).and_return(linguist_repository) + end + + it 'sets the correct colour according to language' do + get(:languages, namespace_id: project.namespace.path, project_id: project.path, id: 'master') + + expected_values.each do |val| + expect(assigns(:languages)).to include(a_hash_including(val)) + end + end + end +end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 742edd8ba3d..d509f0f2b96 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -708,4 +708,82 @@ describe Projects::MergeRequestsController do end end end + + describe 'POST assign_related_issues' do + let(:issue1) { create(:issue, project: project) } + let(:issue2) { create(:issue, project: project) } + + def post_assign_issues + merge_request.update!(description: "Closes #{issue1.to_reference} and #{issue2.to_reference}", + author: user, + source_branch: 'feature', + target_branch: 'master') + + post :assign_related_issues, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: merge_request.iid + end + + it 'shows a flash message on success' do + post_assign_issues + + expect(flash[:notice]).to eq '2 issues have been assigned to you' + end + + it 'correctly pluralizes flash message on success' do + issue2.update!(assignee: user) + + post_assign_issues + + expect(flash[:notice]).to eq '1 issue has been assigned to you' + end + + it 'calls MergeRequests::AssignIssuesService' do + expect(MergeRequests::AssignIssuesService).to receive(:new). + with(project, user, merge_request: merge_request). + and_return(double(execute: { count: 1 })) + + post_assign_issues + end + + it 'is skipped when not signed in' do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + sign_out(:user) + + expect(MergeRequests::AssignIssuesService).not_to receive(:new) + + post_assign_issues + end + end + + describe 'GET ci_environments_status' do + context 'when the environment is from a forked project' do + let!(:forked) { create(:project) } + let!(:environment) { create(:environment, project: forked) } + let!(:deployment) { create(:deployment, environment: environment, sha: forked.commit.id, ref: 'master') } + let(:json_response) { JSON.parse(response.body) } + let(:admin) { create(:admin) } + + let(:merge_request) do + create(:forked_project_link, forked_to_project: forked, + forked_from_project: project) + + create(:merge_request, source_project: forked, target_project: project) + end + + before do + forked.team << [user, :master] + + get :ci_environments_status, + namespace_id: merge_request.project.namespace.to_param, + project_id: merge_request.project.to_param, + id: merge_request.iid, format: 'json' + end + + it 'links to the environment on that project' do + expect(json_response.first['url']).to match /#{forked.path_with_namespace}/ + end + end + end end diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb index a6995145cc1..5e661c2c41d 100644 --- a/spec/controllers/projects/tags_controller_spec.rb +++ b/spec/controllers/projects/tags_controller_spec.rb @@ -17,4 +17,18 @@ describe Projects::TagsController do expect(assigns(:releases)).not_to include(invalid_release) end end + + describe 'GET show' do + before { get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, id: id } + + context "valid tag" do + let(:id) { 'v1.0.0' } + it { is_expected.to respond_with(:success) } + end + + context "invalid tag" do + let(:id) { 'latest' } + it { is_expected.to respond_with(:not_found) } + end + end end diff --git a/spec/factories/boards.rb b/spec/factories/boards.rb index 35c4a0b6f08..ec46146d9b5 100644 --- a/spec/factories/boards.rb +++ b/spec/factories/boards.rb @@ -1,5 +1,10 @@ FactoryGirl.define do factory :board do project factory: :empty_project + + after(:create) do |board| + board.lists.create(list_type: :backlog) + board.lists.create(list_type: :done) + end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 873d3fcb5af..719ef17f57e 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -9,6 +9,9 @@ FactoryGirl.define do namespace creator + # Behaves differently to nil due to cache_has_external_issue_tracker + has_external_issue_tracker false + trait :public do visibility_level Gitlab::VisibilityLevel::PUBLIC end @@ -92,6 +95,8 @@ FactoryGirl.define do end factory :redmine_project, parent: :project do + has_external_issue_tracker true + after :create do |project| project.create_redmine_service( active: true, @@ -105,6 +110,8 @@ FactoryGirl.define do end factory :jira_project, parent: :project do + has_external_issue_tracker true + after :create do |project| project.create_jira_service( active: true, @@ -117,12 +124,4 @@ FactoryGirl.define do ) end end - - 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 26ea06e002b..0fb1608a0a3 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -4,7 +4,8 @@ describe 'Issue Boards', feature: true, js: true do include WaitForAjax include WaitForVueResource - let(:project) { create(:project_with_board, :public) } + let(:project) { create(:empty_project, :public) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let!(:user2) { create(:user) } @@ -17,7 +18,7 @@ describe 'Issue Boards', feature: true, js: true do context 'no lists' do before do - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource expect(page).to have_selector('.board', count: 3) end @@ -34,14 +35,14 @@ describe 'Issue Boards', feature: true, js: true do end it 'creates default lists' do - lists = ['Backlog', 'Development', 'Testing', 'Production', 'Ready', 'Done'] + lists = ['Backlog', 'To Do', 'Doing', 'Done'] page.within(find('.board-blank-state')) do click_button('Add default lists') end wait_for_vue_resource - expect(page).to have_selector('.board', count: 6) + expect(page).to have_selector('.board', count: 4) page.all('.board').each_with_index do |list, i| expect(list.find('.board-title')).to have_content(lists[i]) @@ -60,8 +61,8 @@ describe 'Issue Boards', feature: true, js: true do 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) } + let!(:list1) { create(:list, board: board, label: planning, position: 0) } + let!(:list2) { create(:list, board: board, label: development, position: 1) } let!(:confidential_issue) { create(:issue, :confidential, project: project, author: user) } let!(:issue1) { create(:issue, project: project, assignee: user) } @@ -75,7 +76,7 @@ describe 'Issue Boards', feature: true, js: true do let!(:issue9) { create(:labeled_issue, project: project, labels: [testing, bug, accepting]) } before do - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource @@ -169,7 +170,7 @@ describe 'Issue Boards', feature: true, js: true do create(:issue, project: project) end - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource page.within(find('.board', match: :first)) do @@ -468,7 +469,7 @@ describe 'Issue Boards', feature: true, js: true do it 'removes filtered labels' do wait_for_vue_resource - + page.within '.labels-filter' do click_button('Label') wait_for_ajax @@ -603,7 +604,7 @@ describe 'Issue Boards', feature: true, js: true do context 'keyboard shortcuts' do before do - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource end @@ -616,7 +617,7 @@ describe 'Issue Boards', feature: true, js: true do context 'signed out user' do before do logout - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource end @@ -632,7 +633,7 @@ describe 'Issue Boards', feature: true, js: true do project.team << [user_guest, :guest] logout login_as(user_guest) - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource end diff --git a/spec/features/boards/keyboard_shortcut_spec.rb b/spec/features/boards/keyboard_shortcut_spec.rb index 7ef68e9eb8d..a5fc766401f 100644 --- a/spec/features/boards/keyboard_shortcut_spec.rb +++ b/spec/features/boards/keyboard_shortcut_spec.rb @@ -6,9 +6,7 @@ describe 'Issue Boards shortcut', feature: true, js: true do let(:project) { create(:empty_project) } before do - project.create_board - project.board.lists.create(list_type: :backlog) - project.board.lists.create(list_type: :done) + create(:board, project: project) login_as :admin diff --git a/spec/features/boards/new_issue_spec.rb b/spec/features/boards/new_issue_spec.rb new file mode 100644 index 00000000000..67d6da5f39a --- /dev/null +++ b/spec/features/boards/new_issue_spec.rb @@ -0,0 +1,81 @@ +require 'rails_helper' + +describe 'Issue Boards new issue', feature: true, js: true do + include WaitForAjax + include WaitForVueResource + + let(:project) { create(:empty_project, :public) } + let(:board) { create(:board, project: project) } + let(:user) { create(:user) } + + context 'authorized user' do + before do + project.team << [user, :master] + + login_as(user) + + visit namespace_project_board_path(project.namespace, project, board) + wait_for_vue_resource + + expect(page).to have_selector('.board', count: 3) + end + + it 'displays new issue button' do + expect(page).to have_selector('.board-issue-count-holder .btn', count: 1) + end + + it 'does not display new issue button in done list' do + page.within('.board:nth-child(3)') do + expect(page).not_to have_selector('.board-issue-count-holder .btn') + end + end + + it 'shows form when clicking button' do + page.within(first('.board')) do + find('.board-issue-count-holder .btn').click + + expect(page).to have_selector('.board-new-issue-form') + end + end + + it 'hides form when clicking cancel' do + page.within(first('.board')) do + find('.board-issue-count-holder .btn').click + + expect(page).to have_selector('.board-new-issue-form') + + click_button 'Cancel' + + expect(page).to have_selector('.board-new-issue-form', visible: false) + end + end + + it 'creates new issue' do + page.within(first('.board')) do + find('.board-issue-count-holder .btn').click + end + + page.within(first('.board-new-issue-form')) do + find('.form-control').set('bug') + click_button 'Submit issue' + end + + wait_for_vue_resource + + page.within(first('.board .board-issue-count')) do + expect(page).to have_content('1') + end + end + end + + context 'unauthorized user' do + before do + visit namespace_project_board_path(project.namespace, project, board) + wait_for_vue_resource + end + + it 'does not display new issue button' do + expect(page).to have_selector('.board-issue-count-holder .btn', count: 0) + end + end +end diff --git a/spec/features/compare_spec.rb b/spec/features/compare_spec.rb index 33dfd0d5b62..43eb4000e58 100644 --- a/spec/features/compare_spec.rb +++ b/spec/features/compare_spec.rb @@ -44,7 +44,7 @@ describe "Compare", js: true do 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 + dropdown.fill_in("Filter by Git revision", with: selection) + find_link(selection, visible: true).click end end diff --git a/spec/features/environments_spec.rb b/spec/features/environments_spec.rb index 3b38a7f5007..d0968b3e595 100644 --- a/spec/features/environments_spec.rb +++ b/spec/features/environments_spec.rb @@ -44,6 +44,10 @@ feature 'Environments', feature: true do scenario 'does show deployment SHA' do expect(page).to have_link(deployment.short_sha) end + + scenario 'does show deployment internal id' do + expect(page).to have_content(deployment.iid) + end context 'with build and manual actions' do given(:pipeline) { create(:ci_pipeline, project: project) } @@ -61,6 +65,20 @@ feature 'Environments', feature: true do expect(page).to have_content(manual.name) expect(manual.reload).to be_pending end + + scenario 'does show build name and id' do + expect(page).to have_link("#{build.name} (##{build.id})") + end + + context 'with external_url' do + given(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } + given(:build) { create(:ci_build, pipeline: pipeline) } + given(:deployment) { create(:deployment, environment: environment, deployable: build) } + + scenario 'does show an external link button' do + expect(page).to have_link(nil, href: environment.external_url) + end + end end end end @@ -122,6 +140,16 @@ feature 'Environments', feature: true do expect(page).to have_content(manual.name) expect(manual.reload).to be_pending end + + context 'with external_url' do + given(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } + given(:build) { create(:ci_build, pipeline: pipeline) } + given(:deployment) { create(:deployment, environment: environment, deployable: build) } + + scenario 'does show an external link button' do + expect(page).to have_link(nil, href: environment.external_url) + end + end end end end diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb index 10d3713f19f..d811b05b0c3 100644 --- a/spec/features/groups/members/owner_manages_access_requests_spec.rb +++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb @@ -41,7 +41,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do def expect_visible_access_request(group, user) expect(group.requesters.exists?(user_id: user)).to be_truthy - expect(page).to have_content "#{group.name} access requests 1" + expect(page).to have_content "Users requesting access to #{group.name} 1" expect(page).to have_content user.name end end diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index 2d8b59472e8..13bfe90302c 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -5,43 +5,105 @@ feature 'Group', feature: true do login_as(:admin) end - describe 'creating a group with space in group path' do - it 'renders new group form with validation errors' do - visit new_group_path - fill_in 'Group path', with: 'space group' + matcher :have_namespace_error_message do + match do |page| + page.has_content?("Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.', '.git' or '.atom'.") + end + end + + describe 'create a group' do + before { visit new_group_path } + + describe 'with space in group path' do + it 'renders new group form with validation errors' do + fill_in 'Group path', with: 'space group' + click_button 'Create group' + + expect(current_path).to eq(groups_path) + expect(page).to have_namespace_error_message + end + end + + describe 'with .atom at end of group path' do + it 'renders new group form with validation errors' do + fill_in 'Group path', with: 'atom_group.atom' + click_button 'Create group' + + expect(current_path).to eq(groups_path) + expect(page).to have_namespace_error_message + end + end + + describe 'with .git at end of group path' do + it 'renders new group form with validation errors' do + fill_in 'Group path', with: 'git_group.git' + click_button 'Create group' + + expect(current_path).to eq(groups_path) + expect(page).to have_namespace_error_message + end + end + end + + describe 'group edit' do + let(:group) { create(:group) } + let(:path) { edit_group_path(group) } + let(:new_name) { 'new-name' } + + before { visit path } - click_button 'Create group' + it 'saves new settings' do + fill_in 'group_name', with: new_name + click_button 'Save group' - expect(current_path).to eq(groups_path) - expect(page).to have_content("Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.'.") + expect(page).to have_content 'successfully updated' + expect(find('#group_name').value).to eq(new_name) + + page.within ".navbar-gitlab" do + expect(page).to have_content new_name + end + end + + it 'removes group' do + click_link 'Remove Group' + + expect(page).to have_content "scheduled for deletion" end end - describe 'description' do + describe 'group page with markdown description' do let(:group) { create(:group) } let(:path) { group_path(group) } it 'parses Markdown' do group.update_attribute(:description, 'This is **my** group') + visit path + expect(page).to have_css('.description > p > strong') end it 'passes through html-pipeline' do group.update_attribute(:description, 'This group is the :poop:') + visit path + expect(page).to have_css('.description > p > img') end it 'sanitizes unwanted tags' do group.update_attribute(:description, '# Group Description') + visit path + expect(page).not_to have_css('.description h1') end it 'permits `rel` attribute on links' do group.update_attribute(:description, 'https://google.com/') + visit path + expect(page).to have_css('.description a[rel]') end end diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index 2523b4b7898..996f39ea06d 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -29,7 +29,7 @@ feature 'Login', feature: true do describe 'with two-factor authentication' do def enter_code(code) - fill_in 'Two-Factor Authentication code', with: code + fill_in 'user_otp_attempt', with: code click_button 'Verify code' end diff --git a/spec/features/merge_requests/assign_issues_spec.rb b/spec/features/merge_requests/assign_issues_spec.rb new file mode 100644 index 00000000000..43cc6f2a2a7 --- /dev/null +++ b/spec/features/merge_requests/assign_issues_spec.rb @@ -0,0 +1,51 @@ +require 'rails_helper' + +feature 'Merge request issue assignment', js: true, feature: true do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue1) { create(:issue, project: project) } + let(:issue2) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, :simple, source_project: project, author: user, description: "fixes #{issue1.to_reference} and #{issue2.to_reference}") } + let(:service) { MergeRequests::AssignIssuesService.new(merge_request, user, user, project) } + + before do + project.team << [user, :developer] + end + + def visit_merge_request(current_user = nil) + login_as(current_user || user) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + context 'logged in as author' do + scenario 'updates related issues' do + visit_merge_request + click_link "Assign yourself to these issues" + + expect(page).to have_content "2 issues have been assigned to you" + end + + it 'returns user to the merge request' do + visit_merge_request + click_link "Assign yourself to these issues" + + expect(page).to have_content merge_request.description + end + + it "doesn't display if related issues are already assigned" do + [issue1, issue2].each { |issue| issue.update!(assignee: user) } + + visit_merge_request + + expect(page).not_to have_content "Assign yourself" + end + end + + context 'not MR author' do + it "doesn't not show assignment link" do + visit_merge_request(create(:user)) + + expect(page).not_to have_content "Assign yourself" + end + end +end diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb index 4d5d4aa121a..a506624b30d 100644 --- a/spec/features/merge_requests/created_from_fork_spec.rb +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -45,7 +45,7 @@ feature 'Merge request created from fork' do page.within('.merge-request-tabs') { click_link 'Builds' } wait_for_ajax - page.within('table.builds') do + page.within('table.ci-table') do expect(page).to have_content 'rspec' expect(page).to have_content 'spinach' end diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb index 60bc07bd1a0..bc2b0ff3e2c 100644 --- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -2,18 +2,26 @@ require 'spec_helper' feature 'Merge When Build Succeeds', feature: true, js: true do let(:user) { create(:user) } + let(:project) { create(:project, :public) } - let(:project) { create(:project, :public) } - let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") } + let(:merge_request) do + create(:merge_request_with_diffs, source_project: project, + author: user, + title: 'Bug NS-04') + end - before do - project.team << [user, :master] - project.enable_ci + let(:pipeline) do + create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch) end - context "Active build for Merge Request" do - let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } - let!(:ci_build) { create(:ci_build, pipeline: pipeline) } + before { project.team << [user, :master] } + + context 'when there is active build for merge request' do + background do + create(:ci_build, pipeline: pipeline) + end before do login_as user @@ -41,26 +49,30 @@ feature 'Merge When Build Succeeds', feature: true, js: true do end end - context 'When it is enabled' do + context 'when merge when build succeeds is enabled' do let(:merge_request) do - create(:merge_request_with_diffs, :simple, source_project: project, author: user, - merge_user: user, title: "MepMep", merge_when_build_succeeds: true) + create(:merge_request_with_diffs, :simple, source_project: project, + author: user, + merge_user: user, + title: 'MepMep', + merge_when_build_succeeds: true) end - let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } - let!(:ci_build) { create(:ci_build, pipeline: pipeline) } + let!(:build) do + create(:ci_build, pipeline: pipeline) + end before do login_as user visit_merge_request(merge_request) end - it 'cancels the automatic merge' do + it 'allows to cancel the automatic merge' do click_link "Cancel Automatic Merge" expect(page).to have_button "Merge When Build Succeeds" - visit_merge_request(merge_request) # Needed to refresh the page + visit_merge_request(merge_request) # refresh the page expect(page).to have_content "Canceled the automatic merge" end @@ -70,10 +82,21 @@ feature 'Merge When Build Succeeds', feature: true, js: true do click_link "Remove Source Branch When Merged" expect(page).to have_content "The source branch will be removed" end + + context 'when build succeeds' do + background { build.success } + + it 'merges merge request' do + visit_merge_request(merge_request) # refresh the page + + expect(page).to have_content 'The changes were merged' + expect(merge_request.reload).to be_merged + end + end end - context 'Build is not active' do - it "does not allow for enabling" do + context 'when build is not active' do + it "does not allow to enable merge when build succeeds" do visit_merge_request(merge_request) expect(page).not_to have_link "Merge When Build Succeeds" end diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb index cb3cea3fd51..7b8af555f0e 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -20,7 +20,7 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do login_with(user) visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - + after do wait_for_ajax end @@ -34,7 +34,7 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do expect(page).to have_content 'Your commands have been executed!' expect(merge_request.reload.work_in_progress?).to eq true - end + end it 'removes the WIP: prefix from the title' do merge_request.title = merge_request.wip_title @@ -45,7 +45,7 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do expect(page).to have_content 'Your commands have been executed!' expect(merge_request.reload.work_in_progress?).to eq false - end + end end context 'when the current user cannot toggle the WIP prefix' do diff --git a/spec/features/merge_requests/widget_deployments_spec.rb b/spec/features/merge_requests/widget_deployments_spec.rb new file mode 100644 index 00000000000..8e23ec50d4a --- /dev/null +++ b/spec/features/merge_requests/widget_deployments_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +feature 'Widget Deployments Header', feature: true, js: true do + include WaitForAjax + + describe 'when deployed to an environment' do + let(:project) { merge_request.target_project } + let(:merge_request) { create(:merge_request, :merged) } + let(:environment) { create(:environment, project: project) } + let!(:deployment) do + create(:deployment, environment: environment, sha: project.commit('master').id) + end + + before do + login_as :admin + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'displays that the environment is deployed' do + wait_for_ajax + + expect(page).to have_content("Deployed to #{environment.name}") + expect(find('.ci_widget > span > span')['data-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium)) + end + end +end diff --git a/spec/features/projects/guest_navigation_menu_spec.rb b/spec/features/projects/guest_navigation_menu_spec.rb new file mode 100644 index 00000000000..c22441f8929 --- /dev/null +++ b/spec/features/projects/guest_navigation_menu_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe "Guest navigation menu" do + let(:project) { create :empty_project, :private } + let(:guest) { create :user } + + before do + project.team << [guest, :guest] + + login_as(guest) + end + + it "shows allowed tabs only" do + visit namespace_project_path(project.namespace, project) + + within(".nav-links") do + expect(page).to have_content 'Project' + expect(page).to have_content 'Activity' + expect(page).to have_content 'Issues' + expect(page).to have_content 'Wiki' + + expect(page).not_to have_content 'Repository' + expect(page).not_to have_content 'Pipelines' + expect(page).not_to have_content 'Graphs' + expect(page).not_to have_content 'Merge Requests' + end + end +end diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index cd79c4f512d..d886909ce85 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -15,6 +15,7 @@ feature 'issuable templates', feature: true, js: true do let(:template_content) { 'this is a test "bug" template' } let(:longtemplate_content) { %Q(this\n\n\n\n\nis\n\n\n\n\na\n\n\n\n\nbug\n\n\n\n\ntemplate) } let(:issue) { create(:issue, author: user, assignee: user, project: project) } + let(:description_addition) { ' appending to description' } background do project.repository.commit_file(user, '.gitlab/issue_templates/bug.md', template_content, 'added issue template', 'master', false) @@ -26,7 +27,26 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects "bug" template' do select_template 'bug' wait_for_ajax - preview_template(template_content) + preview_template + save_changes + end + + scenario 'user selects "bug" template and then "no template"' do + select_template 'bug' + wait_for_ajax + select_option 'No template' + wait_for_ajax + preview_template('') + save_changes('') + end + + scenario 'user selects "bug" template, edits description and then selects "reset template"' do + select_template 'bug' + wait_for_ajax + find_field('issue_description').send_keys(description_addition) + preview_template(template_content + description_addition) + select_option 'Reset template' + preview_template save_changes end @@ -37,7 +57,7 @@ feature 'issuable templates', feature: true, js: true do wait_for_ajax end_height = page.evaluate_script('$(".markdown-area").outerHeight()') - + expect(end_height).not_to eq(start_height) end end @@ -75,7 +95,7 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects "feature-proposal" template' do select_template 'feature-proposal' wait_for_ajax - preview_template(template_content) + preview_template save_changes end end @@ -102,25 +122,31 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects template' do select_template 'feature-proposal' wait_for_ajax - preview_template(template_content) + preview_template save_changes end end end end - def preview_template(expected_content) + def preview_template(expected_content = template_content) click_link 'Preview' expect(page).to have_content expected_content + click_link 'Write' end - def save_changes + def save_changes(expected_content = template_content) click_button "Save changes" - expect(page).to have_content template_content + expect(page).to have_content expected_content end def select_template(name) first('.js-issuable-selector').click first('.js-issuable-selector-wrap .dropdown-content a', text: name).click end + + def select_option(name) + first('.js-issuable-selector').click + first('.js-issuable-selector-wrap .dropdown-footer-list a', text: name).click + end end diff --git a/spec/features/projects/members/group_links_spec.rb b/spec/features/projects/members/group_links_spec.rb new file mode 100644 index 00000000000..cc2f695211c --- /dev/null +++ b/spec/features/projects/members/group_links_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +feature 'Projects > Members > Anonymous user sees members', feature: true, js: true do + include WaitForAjax + + let(:user) { create(:user) } + let(:group) { create(:group, :public) } + let(:project) { create(:empty_project, :public) } + + background do + project.team << [user, :master] + @group_link = create(:project_group_link, project: project, group: group) + + login_as(user) + visit namespace_project_project_members_path(project.namespace, project) + end + + it 'updates group access level' do + select 'Guest', from: "member_access_level_#{group.id}" + wait_for_ajax + + visit namespace_project_project_members_path(project.namespace, project) + + expect(page).to have_select("member_access_level_#{group.id}", selected: 'Guest') + end + + it 'updates expiry date' do + tomorrow = Date.today + 3 + + fill_in "member_expires_at_#{group.id}", with: tomorrow.strftime("%F") + wait_for_ajax + + page.within(find('li.group_member')) do + expect(page).to have_content('Expires in') + end + end + + it 'deletes group link' do + page.within(first('.group_member')) do + find('.btn-remove').click + end + wait_for_ajax + + expect(page).not_to have_selector('.group_member') + end + + context 'search' do + it 'finds no results' do + page.within '.member-search-form' do + fill_in 'search', with: 'testing 123' + find('.member-search-btn').click + end + + expect(page).not_to have_selector('.group_member') + end + + it 'finds results' do + page.within '.member-search-form' do + fill_in 'search', with: group.name + find('.member-search-btn').click + end + + expect(page).to have_selector('.group_member', count: 1) + end + end +end diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 430c384ac2e..27a83fdcd1f 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' feature 'Projects > Members > Master adds member with expiration date', feature: true, js: true do + include WaitForAjax include Select2Helper include ActiveSupport::Testing::TimeHelpers @@ -20,7 +21,7 @@ feature 'Projects > Members > Master adds member with expiration date', feature: page.within '.users-project-form' do select2(new_member.id, from: '#user_ids', multiple: true) fill_in 'expires_at', with: '2016-08-10' - click_on 'Add users to project' + click_on 'Add to project' end page.within '.project_member:first-child' do @@ -35,9 +36,8 @@ feature 'Projects > Members > Master adds member with expiration date', feature: visit namespace_project_project_members_path(project.namespace, project) page.within '.project_member:first-child' do - click_on 'Edit' - fill_in 'Access expiration date', with: '2016-08-09' - click_on 'Save' + find('.js-access-expiration-date').set '2016-08-09' + wait_for_ajax expect(page).to have_content('Expires in 3 days') end end diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb index f7fcd9b6731..d15376931c3 100644 --- a/spec/features/projects/members/master_manages_access_requests_spec.rb +++ b/spec/features/projects/members/master_manages_access_requests_spec.rb @@ -41,7 +41,7 @@ feature 'Projects > Members > Master manages access requests', feature: true do def expect_visible_access_request(project, user) expect(project.requesters.exists?(user_id: user)).to be_truthy - expect(page).to have_content "#{project.name} access requests 1" + expect(page).to have_content "Users requesting access to #{project.name} 1" expect(page).to have_content user.name end end diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index ccb5c06dab0..79417c769a8 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -203,7 +203,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for guest } + it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index a752c1d7235..65544f79eba 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -14,7 +14,7 @@ feature 'Signup', feature: true do fill_in 'new_user_username', with: user.username fill_in 'new_user_email', with: user.email fill_in 'new_user_password', with: user.password - click_button "Sign up" + click_button "Register" expect(current_path).to eq users_almost_there_path expect(page).to have_content("Please check your email to confirm your account") @@ -33,7 +33,7 @@ feature 'Signup', feature: true do fill_in 'new_user_username', with: user.username fill_in 'new_user_email', with: user.email fill_in 'new_user_password', with: user.password - click_button "Sign up" + click_button "Register" expect(current_path).to eq dashboard_projects_path expect(page).to have_content("Welcome! You have signed up successfully.") @@ -52,7 +52,7 @@ feature 'Signup', feature: true do fill_in 'new_user_username', with: user.username fill_in 'new_user_email', with: existing_user.email fill_in 'new_user_password', with: user.password - click_button "Sign up" + click_button "Register" expect(current_path).to eq user_registration_path expect(page).to have_content("error prohibited this user from being saved") @@ -69,7 +69,7 @@ feature 'Signup', feature: true do fill_in 'new_user_username', with: user.username fill_in 'new_user_email', with: existing_user.email fill_in 'new_user_password', with: user.password - click_button "Sign up" + click_button "Register" expect(current_path).to eq user_registration_path expect(page.body).not_to match(/#{user.password}/) diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb index ff6933dc8d9..b750f27ea72 100644 --- a/spec/features/u2f_spec.rb +++ b/spec/features/u2f_spec.rb @@ -160,7 +160,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: login_with(user) @u2f_device.respond_to_u2f_authentication - click_on "Login Via U2F Device" + click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" @@ -174,7 +174,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: login_with(user) @u2f_device.respond_to_u2f_authentication - click_on "Login Via U2F Device" + click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" @@ -186,7 +186,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: login_with(user, remember: true) @u2f_device.respond_to_u2f_authentication - click_on "Login Via U2F Device" + click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') within 'div#js-authenticate-u2f' do @@ -209,7 +209,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: # Try authenticating user with the old U2F device login_as(current_user) @u2f_device.respond_to_u2f_authentication - click_on "Login Via U2F Device" + click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" @@ -230,7 +230,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: # Try authenticating user with the same U2F device login_as(current_user) @u2f_device.respond_to_u2f_authentication - click_on "Login Via U2F Device" + click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" @@ -244,7 +244,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: unregistered_device = FakeU2fDevice.new(page, FFaker::Name.first_name) login_as(user) unregistered_device.respond_to_u2f_authentication - click_on "Login Via U2F Device" + click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" @@ -271,7 +271,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: [first_device, second_device].each do |device| login_as(user) device.respond_to_u2f_authentication - click_on "Login Via U2F Device" + click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index b5a94fe0383..ec4c4d62f53 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -1,15 +1,16 @@ require 'spec_helper' -feature 'Users', feature: true do +feature 'Users', feature: true, js: true do let(:user) { create(:user, username: 'user1', name: 'User 1', email: 'user1@gitlab.com') } scenario 'GET /users/sign_in creates a new user account' do visit new_user_session_path + click_link 'Register' fill_in 'new_user_name', with: 'Name Surname' fill_in 'new_user_username', with: 'Great' fill_in 'new_user_email', with: 'name@mail.com' fill_in 'new_user_password', with: 'password1234' - expect { click_button 'Sign up' }.to change { User.count }.by(1) + expect { click_button 'Register' }.to change { User.count }.by(1) end scenario 'Successful user signin invalidates password reset token' do @@ -31,15 +32,49 @@ feature 'Users', feature: true do scenario 'Should show one error if email is already taken' do visit new_user_session_path + click_link 'Register' fill_in 'new_user_name', with: 'Another user name' fill_in 'new_user_username', with: 'anotheruser' fill_in 'new_user_email', with: user.email fill_in 'new_user_password', with: '12341234' - expect { click_button 'Sign up' }.to change { User.count }.by(0) + expect { click_button 'Register' }.to change { User.count }.by(0) expect(page).to have_text('Email has already been taken') expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}' end + describe 'redirect alias routes' do + before { user } + + scenario '/u/user1 redirects to user page' do + visit '/u/user1' + + expect(current_path).to eq user_path(user) + expect(page).to have_text(user.name) + end + end + + feature 'username validation' do + include WaitForAjax + let(:loading_icon) { '.fa.fa-spinner' } + let(:username_input) { 'new_user_username' } + + before(:each) do + visit new_user_session_path + click_link 'Register' + end + scenario 'shows an error border if the username already exists' do + fill_in username_input, with: user.username + wait_for_ajax + expect(find('.username')).to have_css '.gl-field-error-outline' + end + + scenario 'doesn\'t show an error border if the username is available' do + fill_in username_input, with: 'new-user' + wait_for_ajax + expect(find('#new_user_username')).not_to have_css '.gl-field-error-outline' + end + end + def errors_on_page(page) page.find('#error_explanation').find('ul').all('li').map{ |item| item.text }.join("\n") end diff --git a/spec/finders/trending_projects_finder_spec.rb b/spec/finders/trending_projects_finder_spec.rb deleted file mode 100644 index cfe15b9defa..00000000000 --- a/spec/finders/trending_projects_finder_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -require 'spec_helper' - -describe TrendingProjectsFinder do - let(:user) { create(:user) } - let(:public_project1) { create(:empty_project, :public) } - let(:public_project2) { create(:empty_project, :public) } - let(:private_project) { create(:empty_project, :private) } - let(:internal_project) { create(:empty_project, :internal) } - - before do - 3.times do - create(:note_on_commit, project: public_project1) - end - - 2.times do - create(:note_on_commit, project: public_project2, created_at: 5.weeks.ago) - end - - create(:note_on_commit, project: private_project) - create(:note_on_commit, project: internal_project) - end - - describe '#execute', caching: true do - context 'without an explicit time range' do - it 'returns public trending projects' do - projects = described_class.new.execute - - expect(projects).to eq([public_project1]) - end - end - - context 'with an explicit time range' do - it 'returns public trending projects' do - projects = described_class.new.execute(2) - - expect(projects).to eq([public_project1, public_project2]) - end - end - - it 'caches the list of projects' do - projects = described_class.new - - expect(Project).to receive(:trending).once - - 2.times { projects.execute } - end - end -end diff --git a/spec/fixtures/api/schemas/board.json b/spec/fixtures/api/schemas/board.json new file mode 100644 index 00000000000..03aca4a3cc0 --- /dev/null +++ b/spec/fixtures/api/schemas/board.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required" : [ + "id" + ], + "properties" : { + "id": { "type": "integer" }, + "name": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/boards.json b/spec/fixtures/api/schemas/boards.json new file mode 100644 index 00000000000..117564ef77a --- /dev/null +++ b/spec/fixtures/api/schemas/boards.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "board.json" } +} diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb index 157cc4665a2..c6e3c5c2368 100644 --- a/spec/helpers/broadcast_messages_helper_spec.rb +++ b/spec/helpers/broadcast_messages_helper_spec.rb @@ -7,7 +7,7 @@ describe BroadcastMessagesHelper do end it 'includes the current message' do - current = double(message: 'Current Message') + current = BroadcastMessage.new(message: 'Current Message') allow(helper).to receive(:broadcast_message_style).and_return(nil) @@ -15,7 +15,7 @@ describe BroadcastMessagesHelper do end it 'includes custom style' do - current = double(message: 'Current Message') + current = BroadcastMessage.new(message: 'Current Message') allow(helper).to receive(:broadcast_message_style).and_return('foo') diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 67bac782591..abe08d95ece 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -63,28 +63,38 @@ describe IssuesHelper do end describe '#award_user_list' do - let!(:awards) { build_list(:award_emoji, 15) } + it "returns a comma-separated list of the first X users" do + user = build_stubbed(:user, name: 'Joe') + awards = Array.new(3, build_stubbed(:award_emoji, user: user)) - it "returns a comma seperated list of 1-9 users" do - expect(award_user_list(awards.first(9), nil)).to eq(awards.first(9).map { |a| a.user.name }.to_sentence) + expect(award_user_list(awards, nil, limit: 3)) + .to eq('Joe, Joe, and Joe') end it "displays the current user's name as 'You'" do - expect(award_user_list(awards.first(1), awards[0].user)).to eq('You') - end + user = build_stubbed(:user, name: 'Joe') + award = build_stubbed(:award_emoji, user: user) - it "truncates lists of larger than 9 users" do - expect(award_user_list(awards, nil)).to eq(awards.first(9).map { |a| a.user.name }.join(', ') + ", and 6 more.") + expect(award_user_list([award], user)).to eq('You') + expect(award_user_list([award], nil)).to eq 'Joe' end - it "displays the current user in front of 0-9 other users" do - expect(award_user_list(awards, awards[0].user)). - to eq("You, " + awards[1..9].map { |a| a.user.name }.join(', ') + ", and 5 more.") + it "truncates lists" do + user = build_stubbed(:user, name: 'Jane') + awards = Array.new(5, build_stubbed(:award_emoji, user: user)) + + expect(award_user_list(awards, nil, limit: 3)) + .to eq('Jane, Jane, Jane, and 2 more.') end - it "displays the current user in front regardless of position in the list" do - expect(award_user_list(awards, awards[12].user)). - to eq("You, " + awards[0..8].map { |a| a.user.name }.join(', ') + ", and 5 more.") + it "displays the current user in front of other users" do + current_user = build_stubbed(:user) + my_award = build_stubbed(:award_emoji, user: current_user) + award = build_stubbed(:award_emoji, user: build_stubbed(:user, name: 'Jane')) + awards = Array.new(5, award).push(my_award) + + expect(award_user_list(awards, current_user, limit: 2)). + to eq("You, Jane, and 4 more.") end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index bcd53440cb4..8113742923b 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -72,7 +72,7 @@ describe ProjectsHelper do it 'returns an HTML link to the user' do link = helper.link_to_member(project, user) - expect(link).to match(%r{/u/#{user.username}}) + expect(link).to match(%r{/#{user.username}}) end end end diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index c5b5aa8c445..64aa41020c9 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -19,7 +19,7 @@ describe SearchHelper do expect(subject.filename).to eq('CHANGELOG') expect(subject.basename).to eq('CHANGELOG') expect(subject.ref).to eq('master') - expect(subject.startline).to eq(186) + expect(subject.startline).to eq(188) expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n") end diff --git a/spec/javascripts/activities_spec.js.es6 b/spec/javascripts/activities_spec.js.es6 new file mode 100644 index 00000000000..743b15460c6 --- /dev/null +++ b/spec/javascripts/activities_spec.js.es6 @@ -0,0 +1,61 @@ +/*= require jquery.cookie.js */ +/*= require jquery.endless-scroll.js */ +/*= require pager */ +/*= require activities */ + +(() => { + window.gon || (window.gon = {}); + const fixtureTemplate = 'event_filter.html'; + const filters = [ + { + id: 'all', + }, { + id: 'push', + name: 'push events', + }, { + id: 'merged', + name: 'merge events', + }, { + id: 'comments', + },{ + id: 'team', + }]; + + function getEventName(index) { + let filter = filters[index]; + return filter.hasOwnProperty('name') ? filter.name : filter.id; + } + + function getSelector(index) { + let filter = filters[index]; + return `#${filter.id}_event_filter` + } + + describe('Activities', () => { + beforeEach(() => { + fixture.load(fixtureTemplate); + new Activities(); + }); + + for(let i = 0; i < filters.length; i++) { + ((i) => { + describe(`when selecting ${getEventName(i)}`, () => { + beforeEach(() => { + $(getSelector(i)).click(); + }); + + for(let x = 0; x < filters.length; x++) { + ((x) => { + let shouldHighlight = i === x; + let testName = shouldHighlight ? 'should highlight' : 'should not highlight'; + + it(`${testName} ${getEventName(x)}`, () => { + expect($(getSelector(x)).parent().hasClass('active')).toEqual(shouldHighlight); + }); + })(x); + } + }); + })(i); + } + }); +})(); diff --git a/spec/javascripts/boards/boards_store_spec.js.es6 b/spec/javascripts/boards/boards_store_spec.js.es6 index 078e4b00023..15c305ce321 100644 --- a/spec/javascripts/boards/boards_store_spec.js.es6 +++ b/spec/javascripts/boards/boards_store_spec.js.es6 @@ -14,7 +14,7 @@ (() => { beforeEach(() => { - gl.boardService = new BoardService('/test/issue-boards/board'); + gl.boardService = new BoardService('/test/issue-boards/board', '1'); gl.issueBoards.BoardsStore.create(); $.cookie('issue_board_welcome_hidden', 'false'); diff --git a/spec/javascripts/boards/issue_spec.js.es6 b/spec/javascripts/boards/issue_spec.js.es6 index 3569d1b98bd..328c6f82ab5 100644 --- a/spec/javascripts/boards/issue_spec.js.es6 +++ b/spec/javascripts/boards/issue_spec.js.es6 @@ -16,7 +16,7 @@ describe('Issue model', () => { let issue; beforeEach(() => { - gl.boardService = new BoardService('/test/issue-boards/board'); + gl.boardService = new BoardService('/test/issue-boards/board', '1'); gl.issueBoards.BoardsStore.create(); issue = new ListIssue({ diff --git a/spec/javascripts/boards/list_spec.js.es6 b/spec/javascripts/boards/list_spec.js.es6 index 1688b996162..ec78d82e919 100644 --- a/spec/javascripts/boards/list_spec.js.es6 +++ b/spec/javascripts/boards/list_spec.js.es6 @@ -16,7 +16,7 @@ describe('List model', () => { let list; beforeEach(() => { - gl.boardService = new BoardService('/test/issue-boards/board'); + gl.boardService = new BoardService('/test/issue-boards/board', '1'); gl.issueBoards.BoardsStore.create(); list = new List(listObj); diff --git a/spec/javascripts/boards/mock_data.js.es6 b/spec/javascripts/boards/mock_data.js.es6 index f3797ed44d4..052455f2ca6 100644 --- a/spec/javascripts/boards/mock_data.js.es6 +++ b/spec/javascripts/boards/mock_data.js.es6 @@ -26,7 +26,7 @@ const listObjDuplicate = { const BoardsMockData = { 'GET': { - '/test/issue-boards/board/lists{/id}/issues': { + '/test/issue-boards/board/1/lists{/id}/issues': { issues: [{ title: 'Testing', iid: 1, @@ -37,13 +37,13 @@ const BoardsMockData = { } }, 'POST': { - '/test/issue-boards/board/lists{/id}': listObj + '/test/issue-boards/board/1/lists{/id}': listObj }, 'PUT': { - '/test/issue-boards/board/lists{/id}': {} + '/test/issue-boards/board/1/lists{/id}': {} }, 'DELETE': { - '/test/issue-boards/board/lists{/id}': {} + '/test/issue-boards/board/1/lists{/id}': {} } }; diff --git a/spec/javascripts/fixtures/event_filter.html.haml b/spec/javascripts/fixtures/event_filter.html.haml new file mode 100644 index 00000000000..95e248cadf8 --- /dev/null +++ b/spec/javascripts/fixtures/event_filter.html.haml @@ -0,0 +1,21 @@ +%ul.nav-links.event-filter.scrolling-tabs + %li.active + %a.event-filter-link{ id: "all_event_filter", title: "Filter by all", href: "/dashboard/activity"} + %span + All + %li + %a.event-filter-link{ id: "push_event_filter", title: "Filter by push events", href: "/dashboard/activity"} + %span + Push events + %li + %a.event-filter-link{ id: "merged_event_filter", title: "Filter by merge events", href: "/dashboard/activity"} + %span + Merge events + %li + %a.event-filter-link{ id: "comments_event_filter", title: "Filter by comments", href: "/dashboard/activity"} + %span + Comments + %li + %a.event-filter-link{ id: "team_event_filter", title: "Filter by team", href: "/dashboard/activity"} + %span + Team
\ No newline at end of file diff --git a/spec/javascripts/fixtures/gl_field_errors.html.haml b/spec/javascripts/fixtures/gl_field_errors.html.haml new file mode 100644 index 00000000000..2526e5e33a5 --- /dev/null +++ b/spec/javascripts/fixtures/gl_field_errors.html.haml @@ -0,0 +1,15 @@ +%form.show-gl-field-errors{action: 'submit', method: 'post'} + .form-group + %input.required-text{required: true, type: 'text'} Text + .form-group + %input.email{type: 'email', title: 'Please provide a valid email address.', required: true } Email + .form-group + %input.password{type: 'password', required: true} Password + .form-group + %input.alphanumeric{type: 'text', pattern: '[a-zA-Z0-9]', required: true} Alphanumeric + .form-group + %input.hidden{ type:'hidden' } + .form-group + %input.custom.no-gl-field-errors{ type:'text' } Custom, do not validate + .form-group + %input.submit{type: 'submit'} Submit diff --git a/spec/javascripts/gl_field_errors_spec.js.es6 b/spec/javascripts/gl_field_errors_spec.js.es6 new file mode 100644 index 00000000000..36feb2b2aa5 --- /dev/null +++ b/spec/javascripts/gl_field_errors_spec.js.es6 @@ -0,0 +1,111 @@ +//= require jquery +//= require gl_field_errors + +((global) => { + fixture.preload('gl_field_errors.html'); + + describe('GL Style Field Errors', function() { + beforeEach(function() { + fixture.load('gl_field_errors.html'); + const $form = this.$form = $('form.show-gl-field-errors'); + this.fieldErrors = new global.GlFieldErrors($form); + }); + + it('should properly initialize the form', function() { + expect(this.$form).toBeDefined(); + expect(this.$form.length).toBe(1); + expect(this.fieldErrors).toBeDefined(); + const inputs = this.fieldErrors.state.inputs; + expect(inputs.length).toBe(5); + }); + + it('should ignore elements with custom error handling', function() { + const customErrorFlag = 'no-gl-field-errors'; + const customErrorElem = $(`.${customErrorFlag}`); + + expect(customErrorElem.length).toBe(1); + + const customErrors = this.fieldErrors.state.inputs.filter((input) => { + return input.inputElement.hasClass(customErrorFlag); + }); + expect(customErrors.length).toBe(0); + }); + + it('should not show any errors before submit attempt', function() { + this.$form.find('.email').val('not-a-valid-email').keyup(); + this.$form.find('.text-required').val('').keyup(); + this.$form.find('.alphanumberic').val('?---*').keyup(); + + const errorsShown = this.$form.find('.gl-field-error-outline'); + expect(errorsShown.length).toBe(0); + }); + + it('should show errors when input valid is submitted', function() { + this.$form.find('.email').val('not-a-valid-email').keyup(); + this.$form.find('.text-required').val('').keyup(); + this.$form.find('.alphanumberic').val('?---*').keyup(); + + this.$form.submit(); + + const errorsShown = this.$form.find('.gl-field-error-outline'); + expect(errorsShown.length).toBe(4); + }); + + it('should properly track validity state on input after invalid submission attempt', function() { + this.$form.submit(); + + const emailInputModel = this.fieldErrors.state.inputs[1]; + const fieldState = emailInputModel.state; + const emailInputElement = emailInputModel.inputElement; + + // No input + expect(emailInputElement).toHaveClass('gl-field-error-outline'); + expect(fieldState.empty).toBe(true); + expect(fieldState.valid).toBe(false); + + // Then invalid input + emailInputElement.val('not-a-valid-email').keyup(); + expect(emailInputElement).toHaveClass('gl-field-error-outline'); + expect(fieldState.empty).toBe(false); + expect(fieldState.valid).toBe(false); + + // Then valid input + emailInputElement.val('email@gitlab.com').keyup(); + expect(emailInputElement).not.toHaveClass('gl-field-error-outline'); + expect(fieldState.empty).toBe(false); + expect(fieldState.valid).toBe(true); + + // Then invalid input + emailInputElement.val('not-a-valid-email').keyup(); + expect(emailInputElement).toHaveClass('gl-field-error-outline'); + expect(fieldState.empty).toBe(false); + expect(fieldState.valid).toBe(false); + + // Then empty input + emailInputElement.val('').keyup(); + expect(emailInputElement).toHaveClass('gl-field-error-outline'); + expect(fieldState.empty).toBe(true); + expect(fieldState.valid).toBe(false); + + // Then valid input + emailInputElement.val('email@gitlab.com').keyup(); + expect(emailInputElement).not.toHaveClass('gl-field-error-outline'); + expect(fieldState.empty).toBe(false); + expect(fieldState.valid).toBe(true); + }); + + it('should properly infer error messages', function() { + this.$form.submit(); + const trackedInputs = this.fieldErrors.state.inputs; + const inputHasTitle = trackedInputs[1]; + const hasTitleErrorElem = inputHasTitle.inputElement.siblings('.gl-field-error'); + const inputNoTitle = trackedInputs[2]; + const noTitleErrorElem = inputNoTitle.inputElement.siblings('.gl-field-error'); + + expect(noTitleErrorElem.text()).toBe('This field is required.'); + expect(hasTitleErrorElem.text()).toBe('Please provide a valid email address.'); + }); + + }); + +})(window.gl || (window.gl = {})); diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 395032a7416..96ee5235acf 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,5 +1,6 @@ /*= require merge_request_tabs */ +//= require breakpoints (function() { describe('MergeRequestTabs', function() { diff --git a/spec/javascripts/merge_request_widget_spec.js b/spec/javascripts/merge_request_widget_spec.js index 17b32914ec3..c9175e2b704 100644 --- a/spec/javascripts/merge_request_widget_spec.js +++ b/spec/javascripts/merge_request_widget_spec.js @@ -1,5 +1,5 @@ - /*= require merge_request_widget */ +/*= require lib/utils/jquery.timeago.js */ (function() { describe('MergeRequestWidget', function() { @@ -8,6 +8,7 @@ window.notify = function() {}; this.opts = { ci_status_url: "http://sampledomain.local/ci/getstatus", + ci_environments_status_url: "http://sampledomain.local/ci/getenvironmentsstatus", ci_status: "", ci_message: { normal: "Build {{status}} for \"{{title}}\"", @@ -20,17 +21,48 @@ gitlab_icon: "gitlab_logo.png", builds_path: "http://sampledomain.local/sampleBuildsPath" }; - this["class"] = new MergeRequestWidget(this.opts); - return this.ciStatusData = { - "title": "Sample MR title", - "sha": "12a34bc5", - "status": "success", - "coverage": 98 - }; + this["class"] = new window.gl.MergeRequestWidget(this.opts); }); + + describe('getCIEnvironmentsStatus', function() { + beforeEach(function() { + this.ciEnvironmentsStatusData = [{ + created_at: '2016-09-12T13:38:30.636Z', + environment_id: 1, + environment_name: 'env1', + external_url: 'https://test-url.com', + external_url_formatted: 'test-url.com' + }]; + + spyOn(jQuery, 'getJSON').and.callFake((req, cb) => { + cb(this.ciEnvironmentsStatusData); + }); + }); + + it('should call renderEnvironments when the environments property is set', function() { + const spy = spyOn(this.class, 'renderEnvironments').and.stub(); + this.class.getCIEnvironmentsStatus(); + expect(spy).toHaveBeenCalledWith(this.ciEnvironmentsStatusData); + }); + + it('should not call renderEnvironments when the environments property is not set', function() { + this.ciEnvironmentsStatusData = null; + const spy = spyOn(this.class, 'renderEnvironments').and.stub(); + this.class.getCIEnvironmentsStatus(); + expect(spy).not.toHaveBeenCalled(); + }); + }); + return describe('getCIStatus', function() { beforeEach(function() { - return spyOn(jQuery, 'getJSON').and.callFake((function(_this) { + this.ciStatusData = { + "title": "Sample MR title", + "sha": "12a34bc5", + "status": "success", + "coverage": 98 + }; + + spyOn(jQuery, 'getJSON').and.callFake((function(_this) { return function(req, cb) { return cb(_this.ciStatusData); }; @@ -61,10 +93,10 @@ this["class"].getCIStatus(false); return expect(spy).not.toHaveBeenCalled(); }); - return it('should not display a notification on the first check after the widget has been created', function() { + it('should not display a notification on the first check after the widget has been created', function() { var spy; spy = spyOn(window, 'notify'); - this["class"] = new MergeRequestWidget(this.opts); + this["class"] = new window.gl.MergeRequestWidget(this.opts); this["class"].getCIStatus(true); return expect(spy).not.toHaveBeenCalled(); }); diff --git a/spec/javascripts/search_autocomplete_spec.js b/spec/javascripts/search_autocomplete_spec.js index 4470fbcb099..333128782a2 100644 --- a/spec/javascripts/search_autocomplete_spec.js +++ b/spec/javascripts/search_autocomplete_spec.js @@ -5,6 +5,8 @@ /*= require lib/utils/common_utils */ /*= require lib/utils/type_utility */ /*= require fuzzaldrin-plus */ +/*= require turbolinks */ +/*= require jquery.turbolinks */ (function() { var addBodyAttributes, assertLinks, dashboardIssuesPath, dashboardMRsPath, groupIssuesPath, groupMRsPath, groupName, mockDashboardOptions, mockGroupOptions, mockProjectOptions, projectIssuesPath, projectMRsPath, projectName, userId, widget; @@ -138,7 +140,7 @@ list = widget.wrap.find('.dropdown-menu').find('ul'); return assertLinks(list, projectIssuesPath, projectMRsPath); }); - return it('should not show category related menu if there is text in the input', function() { + it('should not show category related menu if there is text in the input', function() { var link, list; addBodyAttributes('project'); mockProjectOptions(); @@ -148,6 +150,23 @@ link = "a[href='" + projectIssuesPath + "/?assignee_id=" + userId + "']"; return expect(list.find(link).length).toBe(0); }); + return it('should not submit the search form when selecting an autocomplete row with the keyboard', function() { + var ENTER = 13; + var DOWN = 40; + addBodyAttributes(); + mockDashboardOptions(true); + var submitSpy = spyOnEvent('form', 'submit'); + widget.searchInput.focus(); + widget.wrap.trigger($.Event('keydown', { which: DOWN })); + var enterKeyEvent = $.Event('keydown', { which: ENTER }); + widget.searchInput.trigger(enterKeyEvent); + // This does not currently catch failing behavior. For security reasons, + // browsers will not trigger default behavior (form submit, in this + // example) on JavaScript-created keypresses. + expect(submitSpy).not.toHaveBeenTriggered(); + // Does a worse job at capturing the intent of the test, but works. + expect(enterKeyEvent.isDefaultPrevented()).toBe(true); + }); }); }).call(this); diff --git a/spec/javascripts/u2f/authenticate_spec.js b/spec/javascripts/u2f/authenticate_spec.js index 7ce3884f844..784b43d4846 100644 --- a/spec/javascripts/u2f/authenticate_spec.js +++ b/spec/javascripts/u2f/authenticate_spec.js @@ -21,7 +21,7 @@ setupButton = this.container.find("#js-login-u2f-device"); setupMessage = this.container.find("p"); expect(setupMessage.text()).toContain('Insert your security key'); - expect(setupButton.text()).toBe('Login Via U2F Device'); + expect(setupButton.text()).toBe('Sign in via U2F device'); setupButton.trigger('click'); inProgressMessage = this.container.find("p"); expect(inProgressMessage.text()).toContain("Trying to communicate with your device"); diff --git a/spec/lib/banzai/filter/emoji_filter_spec.rb b/spec/lib/banzai/filter/emoji_filter_spec.rb index b5b38cf0c8c..c8e62f528df 100644 --- a/spec/lib/banzai/filter/emoji_filter_spec.rb +++ b/spec/lib/banzai/filter/emoji_filter_spec.rb @@ -12,11 +12,16 @@ describe Banzai::Filter::EmojiFilter, lib: true do ActionController::Base.asset_host = @original_asset_host end - it 'replaces supported emoji' do + it 'replaces supported name emoji' do doc = filter('<p>:heart:</p>') expect(doc.css('img').first.attr('src')).to eq 'https://foo.com/assets/2764.png' end + it 'replaces supported unicode emoji' do + doc = filter('<p>❤️</p>') + expect(doc.css('img').first.attr('src')).to eq 'https://foo.com/assets/2764.png' + end + it 'ignores unsupported emoji' do exp = act = '<p>:foo:</p>' doc = filter(act) @@ -28,46 +33,96 @@ describe Banzai::Filter::EmojiFilter, lib: true do expect(doc.css('img').first.attr('src')).to eq 'https://foo.com/assets/1F44D.png' end + it 'correctly encodes unicode to the URL' do + doc = filter('<p>👍</p>') + expect(doc.css('img').first.attr('src')).to eq 'https://foo.com/assets/1F44D.png' + end + it 'matches at the start of a string' do doc = filter(':+1:') expect(doc.css('img').size).to eq 1 end + it 'unicode matches at the start of a string' do + doc = filter("'👍'") + expect(doc.css('img').size).to eq 1 + end + it 'matches at the end of a string' do doc = filter('This gets a :-1:') expect(doc.css('img').size).to eq 1 end + it 'unicode matches at the end of a string' do + doc = filter('This gets a 👍') + expect(doc.css('img').size).to eq 1 + end + it 'matches with adjacent text' do doc = filter('+1 (:+1:)') expect(doc.css('img').size).to eq 1 end + it 'unicode matches with adjacent text' do + doc = filter('+1 (👍)') + expect(doc.css('img').size).to eq 1 + end + it 'matches multiple emoji in a row' do doc = filter(':see_no_evil::hear_no_evil::speak_no_evil:') expect(doc.css('img').size).to eq 3 end + it 'unicode matches multiple emoji in a row' do + doc = filter("'🙈🙉🙊'") + expect(doc.css('img').size).to eq 3 + end + + it 'mixed matches multiple emoji in a row' do + doc = filter("'🙈:see_no_evil:🙉:hear_no_evil:🙊:speak_no_evil:'") + expect(doc.css('img').size).to eq 6 + end + it 'has a title attribute' do doc = filter(':-1:') expect(doc.css('img').first.attr('title')).to eq ':-1:' end + it 'unicode has a title attribute' do + doc = filter("'👎'") + expect(doc.css('img').first.attr('title')).to eq ':thumbsdown:' + end + it 'has an alt attribute' do doc = filter(':-1:') expect(doc.css('img').first.attr('alt')).to eq ':-1:' end + it 'unicode has an alt attribute' do + doc = filter("'👎'") + expect(doc.css('img').first.attr('alt')).to eq ':thumbsdown:' + end + it 'has an align attribute' do doc = filter(':8ball:') expect(doc.css('img').first.attr('align')).to eq 'absmiddle' end + it 'unicode has an align attribute' do + doc = filter("'🎱'") + expect(doc.css('img').first.attr('align')).to eq 'absmiddle' + end + it 'has an emoji class' do doc = filter(':cat:') expect(doc.css('img').first.attr('class')).to eq 'emoji' end + it 'unicode has an emoji class' do + doc = filter("'🐱'") + expect(doc.css('img').first.attr('class')).to eq 'emoji' + end + it 'has height and width attributes' do doc = filter(':dog:') img = doc.css('img').first @@ -76,12 +131,26 @@ describe Banzai::Filter::EmojiFilter, lib: true do expect(img.attr('height')).to eq '20' end + it 'unicode has height and width attributes' do + doc = filter("'🐶'") + img = doc.css('img').first + + expect(img.attr('width')).to eq '20' + expect(img.attr('height')).to eq '20' + end + it 'keeps whitespace intact' do doc = filter('This deserves a :+1:, big time.') expect(doc.to_html).to match(/^This deserves a <img.+>, big time\.\z/) end + it 'unicode keeps whitespace intact' do + doc = filter('This deserves a 🎱, big time.') + + expect(doc.to_html).to match(/^This deserves a <img.+>, big time\.\z/) + end + it 'uses a custom asset_root context' do root = Gitlab.config.gitlab.url + 'gitlab/root' @@ -95,4 +164,18 @@ describe Banzai::Filter::EmojiFilter, lib: true do doc = filter(':frowning:', asset_host: 'https://this-is-ignored-i-guess?') expect(doc.css('img').first.attr('src')).to start_with('https://cdn.example.com') end + + it 'uses a custom asset_root context' do + root = Gitlab.config.gitlab.url + 'gitlab/root' + + doc = filter("'🎱'", asset_root: root) + expect(doc.css('img').first.attr('src')).to start_with(root) + end + + it 'uses a custom asset_host context' do + ActionController::Base.asset_host = 'https://cdn.example.com' + + doc = filter("'🎱'", asset_host: 'https://this-is-ignored-i-guess?') + expect(doc.css('img').first.attr('src')).to start_with('https://cdn.example.com') + end end diff --git a/spec/lib/banzai/filter/html_entity_filter_spec.rb b/spec/lib/banzai/filter/html_entity_filter_spec.rb new file mode 100644 index 00000000000..4c68ce6d6e4 --- /dev/null +++ b/spec/lib/banzai/filter/html_entity_filter_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe Banzai::Filter::HtmlEntityFilter, lib: true do + include FilterSpecHelper + + let(:unescaped) { 'foo <strike attr="foo">&&&</strike>' } + let(:escaped) { 'foo <strike attr="foo">&&&</strike>' } + + it 'converts common entities to their HTML-escaped equivalents' do + output = filter(unescaped) + + expect(output).to eq(escaped) + end +end diff --git a/spec/lib/banzai/note_renderer_spec.rb b/spec/lib/banzai/note_renderer_spec.rb index 98f76f36fd5..49556074278 100644 --- a/spec/lib/banzai/note_renderer_spec.rb +++ b/spec/lib/banzai/note_renderer_spec.rb @@ -12,8 +12,7 @@ describe Banzai::NoteRenderer do with(project, user, requested_path: 'foo', project_wiki: wiki, - ref: 'bar', - pipeline: :note). + ref: 'bar'). and_call_original expect_any_instance_of(Banzai::ObjectRenderer). diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index bcdb95250ca..90da78a67dd 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -4,10 +4,18 @@ describe Banzai::ObjectRenderer do let(:project) { create(:empty_project) } let(:user) { project.owner } + def fake_object(attrs = {}) + object = double(attrs.merge("new_record?" => true, "destroyed?" => true)) + allow(object).to receive(:markdown_cache_field_for).with(:note).and_return(:note_html) + allow(object).to receive(:banzai_render_context).with(:note).and_return(project: nil, author: nil) + allow(object).to receive(:update_column).with(:note_html, anything).and_return(true) + object + end + describe '#render' do it 'renders and redacts an Array of objects' do renderer = described_class.new(project, user) - object = double(:object, note: 'hello', note_html: nil) + object = fake_object(note: 'hello', note_html: nil) expect(renderer).to receive(:render_objects).with([object], :note). and_call_original @@ -16,7 +24,7 @@ describe Banzai::ObjectRenderer do with(an_instance_of(Array)). and_call_original - expect(object).to receive(:note_html=).with('<p>hello</p>') + expect(object).to receive(:redacted_note_html=).with('<p>hello</p>') expect(object).to receive(:user_visible_reference_count=).with(0) renderer.render([object], :note) @@ -25,7 +33,7 @@ describe Banzai::ObjectRenderer do describe '#render_objects' do it 'renders an Array of objects' do - object = double(:object, note: 'hello') + object = fake_object(note: 'hello', note_html: nil) renderer = described_class.new(project, user) @@ -57,49 +65,29 @@ describe Banzai::ObjectRenderer do end describe '#context_for' do - let(:object) { double(:object, note: 'hello') } + let(:object) { fake_object(note: 'hello') } let(:renderer) { described_class.new(project, user) } it 'returns a Hash' do expect(renderer.context_for(object, :note)).to be_an_instance_of(Hash) end - it 'includes the cache key' do + it 'includes the banzai render context for the object' do + expect(object).to receive(:banzai_render_context).with(:note).and_return(foo: :bar) context = renderer.context_for(object, :note) - - expect(context[:cache_key]).to eq([object, :note]) - end - - context 'when the object responds to "author"' do - it 'includes the author in the context' do - expect(object).to receive(:author).and_return('Alice') - - context = renderer.context_for(object, :note) - - expect(context[:author]).to eq('Alice') - end - end - - context 'when the object does not respond to "author"' do - it 'does not include the author in the context' do - context = renderer.context_for(object, :note) - - expect(context.key?(:author)).to eq(false) - end + expect(context).to have_key(:foo) + expect(context[:foo]).to eq(:bar) end end describe '#render_attributes' do it 'renders the attribute of a list of objects' do - objects = [double(:doc, note: 'hello'), double(:doc, note: 'bye')] - renderer = described_class.new(project, user, pipeline: :note) + objects = [fake_object(note: 'hello', note_html: nil), fake_object(note: 'bye', note_html: nil)] + renderer = described_class.new(project, user) - expect(Banzai).to receive(:cache_collection_render). - with([ - { text: 'hello', context: renderer.context_for(objects[0], :note) }, - { text: 'bye', context: renderer.context_for(objects[1], :note) } - ]). - and_call_original + objects.each do |object| + expect(Banzai).to receive(:render_field).with(object, :note).and_call_original + end docs = renderer.render_attributes(objects, :note) @@ -114,17 +102,13 @@ describe Banzai::ObjectRenderer do objects = [] renderer = described_class.new(project, user, pipeline: :note) - expect(Banzai).to receive(:cache_collection_render). - with([]). - and_call_original - expect(renderer.render_attributes(objects, :note)).to eq([]) end end describe '#base_context' do let(:context) do - described_class.new(project, user, pipeline: :note).base_context + described_class.new(project, user, foo: :bar).base_context end it 'returns a Hash' do @@ -132,7 +116,7 @@ describe Banzai::ObjectRenderer do end it 'includes the custom attributes' do - expect(context[:pipeline]).to eq(:note) + expect(context[:foo]).to eq(:bar) end it 'includes the current user' do diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb new file mode 100644 index 00000000000..aaa6b12e67e --- /dev/null +++ b/spec/lib/banzai/renderer_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe Banzai::Renderer do + def expect_render(project = :project) + expected_context = { project: project } + expect(renderer).to receive(:cacheless_render) { :html }.with(:markdown, expected_context) + end + + def expect_cache_update + expect(object).to receive(:update_column).with("field_html", :html) + end + + def fake_object(*features) + markdown = :markdown if features.include?(:markdown) + html = :html if features.include?(:html) + + object = double( + "object", + banzai_render_context: { project: :project }, + field: markdown, + field_html: html + ) + + allow(object).to receive(:markdown_cache_field_for).with(:field).and_return("field_html") + allow(object).to receive(:new_record?).and_return(features.include?(:new)) + allow(object).to receive(:destroyed?).and_return(features.include?(:destroyed)) + + object + end + + describe "#render_field" do + let(:renderer) { Banzai::Renderer } + let(:subject) { renderer.render_field(object, :field) } + + context "with an empty cache" do + let(:object) { fake_object(:markdown) } + it "caches and returns the result" do + expect_render + expect_cache_update + expect(subject).to eq(:html) + end + end + + context "with a filled cache" do + let(:object) { fake_object(:markdown, :html) } + + it "uses the cache" do + expect_render.never + expect_cache_update.never + should eq(:html) + end + end + + context "new object" do + let(:object) { fake_object(:new, :markdown) } + + it "doesn't cache the result" do + expect_render + expect_cache_update.never + expect(subject).to eq(:html) + end + end + + context "destroyed object" do + let(:object) { fake_object(:destroyed, :markdown) } + + it "doesn't cache the result" do + expect_render + expect_cache_update.never + expect(subject).to eq(:html) + end + end + end +end diff --git a/spec/lib/constraints/group_url_constrainer_spec.rb b/spec/lib/constraints/group_url_constrainer_spec.rb new file mode 100644 index 00000000000..f0b75a664f2 --- /dev/null +++ b/spec/lib/constraints/group_url_constrainer_spec.rb @@ -0,0 +1,10 @@ +require 'spec_helper' + +describe GroupUrlConstrainer, lib: true do + let!(:username) { create(:group, path: 'gitlab-org') } + + describe '#find_resource' do + it { expect(!!subject.find_resource('gitlab-org')).to be_truthy } + it { expect(!!subject.find_resource('gitlab-com')).to be_falsey } + end +end diff --git a/spec/lib/constraints/namespace_url_constrainer_spec.rb b/spec/lib/constraints/namespace_url_constrainer_spec.rb new file mode 100644 index 00000000000..a5feaacb8ee --- /dev/null +++ b/spec/lib/constraints/namespace_url_constrainer_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe NamespaceUrlConstrainer, lib: true do + let!(:group) { create(:group, path: 'gitlab') } + + describe '#matches?' do + context 'existing namespace' do + it { expect(subject.matches?(request '/gitlab')).to be_truthy } + it { expect(subject.matches?(request '/gitlab.atom')).to be_truthy } + it { expect(subject.matches?(request '/gitlab/')).to be_truthy } + it { expect(subject.matches?(request '//gitlab/')).to be_truthy } + end + + context 'non-existing namespace' do + it { expect(subject.matches?(request '/gitlab-ce')).to be_falsey } + it { expect(subject.matches?(request '/gitlab.ce')).to be_falsey } + it { expect(subject.matches?(request '/g/gitlab')).to be_falsey } + it { expect(subject.matches?(request '/.gitlab')).to be_falsey } + end + end + + def request(path) + OpenStruct.new(path: path) + end +end diff --git a/spec/lib/constraints/user_url_constrainer_spec.rb b/spec/lib/constraints/user_url_constrainer_spec.rb new file mode 100644 index 00000000000..4b26692672f --- /dev/null +++ b/spec/lib/constraints/user_url_constrainer_spec.rb @@ -0,0 +1,10 @@ +require 'spec_helper' + +describe UserUrlConstrainer, lib: true do + let!(:username) { create(:user, username: 'dz') } + + describe '#find_resource' do + it { expect(!!subject.find_resource('dz')).to be_truthy } + it { expect(!!subject.find_resource('john')).to be_falsey } + end +end diff --git a/spec/lib/event_filter_spec.rb b/spec/lib/event_filter_spec.rb new file mode 100644 index 00000000000..a6d8e6927e0 --- /dev/null +++ b/spec/lib/event_filter_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe EventFilter, lib: true do + describe '#apply_filter' do + let(:source_user) { create(:user) } + let!(:public_project) { create(:project, :public) } + + let!(:push_event) { create(:event, action: Event::PUSHED, project: public_project, target: public_project, author: source_user) } + let!(:merged_event) { create(:event, action: Event::MERGED, project: public_project, target: public_project, author: source_user) } + let!(:comments_event) { create(:event, action: Event::COMMENTED, project: public_project, target: public_project, author: source_user) } + let!(:joined_event) { create(:event, action: Event::JOINED, project: public_project, target: public_project, author: source_user) } + let!(:left_event) { create(:event, action: Event::LEFT, project: public_project, target: public_project, author: source_user) } + + it 'applies push filter' do + events = EventFilter.new(EventFilter.push).apply_filter(Event.all) + expect(events).to contain_exactly(push_event) + end + + it 'applies merged filter' do + events = EventFilter.new(EventFilter.merged).apply_filter(Event.all) + expect(events).to contain_exactly(merged_event) + end + + it 'applies comments filter' do + events = EventFilter.new(EventFilter.comments).apply_filter(Event.all) + expect(events).to contain_exactly(comments_event) + end + + it 'applies team filter' do + events = EventFilter.new(EventFilter.team).apply_filter(Event.all) + expect(events).to contain_exactly(joined_event, left_event) + end + + it 'applies all filter' do + events = EventFilter.new(EventFilter.all).apply_filter(Event.all) + expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event) + end + + it 'applies no filter' do + events = EventFilter.new(nil).apply_filter(Event.all) + expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event) + end + + it 'applies unknown filter' do + events = EventFilter.new('').apply_filter(Event.all) + expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event) + end + end +end diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index e10c1f5c547..0e85e302f29 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -6,6 +6,7 @@ describe ExtractsPath, lib: true do include Gitlab::Routing.url_helpers let(:project) { double('project') } + let(:request) { double('request') } before do @project = project @@ -15,9 +16,10 @@ describe ExtractsPath, lib: true do allow(project).to receive(:repository).and_return(repo) allow(project).to receive(:path_with_namespace). and_return('gitlab/gitlab-ci') + allow(request).to receive(:format=) end - describe '#assign_ref' do + describe '#assign_ref_vars' do let(:ref) { sample_commit[:id] } let(:params) { { path: sample_commit[:line_code_path], ref: ref } } @@ -61,6 +63,75 @@ describe ExtractsPath, lib: true do expect(@id).to eq(get_id) end end + + context 'ref only exists without .atom suffix' do + context 'with a path' do + let(:params) { { ref: 'v1.0.0.atom', path: 'README.md' } } + + it 'renders a 404' do + expect(self).to receive(:render_404) + + assign_ref_vars + end + end + + context 'without a path' do + let(:params) { { ref: 'v1.0.0.atom' } } + before { assign_ref_vars } + + it 'sets the un-suffixed version as @ref' do + expect(@ref).to eq('v1.0.0') + end + + it 'sets the request format to Atom' do + expect(request).to have_received(:format=).with(:atom) + end + end + end + + context 'ref exists with .atom suffix' do + context 'with a path' do + let(:params) { { ref: 'master.atom', path: 'README.md' } } + + before do + repository = @project.repository + allow(repository).to receive(:commit).and_call_original + allow(repository).to receive(:commit).with('master.atom').and_return(repository.commit('master')) + + assign_ref_vars + end + + it 'sets the suffixed version as @ref' do + expect(@ref).to eq('master.atom') + end + + it 'does not change the request format' do + expect(request).not_to have_received(:format=) + end + end + + context 'without a path' do + let(:params) { { ref: 'master.atom' } } + + before do + repository = @project.repository + allow(repository).to receive(:commit).and_call_original + allow(repository).to receive(:commit).with('master.atom').and_return(repository.commit('master')) + end + + it 'sets the suffixed version as @ref' do + assign_ref_vars + + expect(@ref).to eq('master.atom') + end + + it 'does not change the request format' do + expect(request).not_to receive(:format=) + + assign_ref_vars + end + end + end end describe '#extract_ref' do @@ -115,4 +186,18 @@ describe ExtractsPath, lib: true do end end end + + describe '#extract_ref_without_atom' do + it 'ignores any matching refs suffixed with atom' do + expect(extract_ref_without_atom('master.atom')).to eq('master') + end + + it 'returns the longest matching ref' do + expect(extract_ref_without_atom('release/app/v1.0.0.atom')).to eq('release/app/v1.0.0') + end + + it 'returns nil if there are no matching refs' do + expect(extract_ref_without_atom('foo.atom')).to eq(nil) + end + end end diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb index 07407f212aa..f826d0d1b04 100644 --- a/spec/lib/gitlab/backend/shell_spec.rb +++ b/spec/lib/gitlab/backend/shell_spec.rb @@ -22,15 +22,15 @@ describe Gitlab::Shell, lib: true do it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") } - describe 'generate_and_link_secret_token' do + describe 'memoized secret_token' do let(:secret_file) { 'tmp/tests/.secret_shell_test' } let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' } before do - allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test') allow(Gitlab.config.gitlab_shell).to receive(:secret_file).and_return(secret_file) + allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test') FileUtils.mkdir('tmp/tests/shell-secret-test') - gitlab_shell.generate_and_link_secret_token + Gitlab::Shell.ensure_secret_token! end after do @@ -39,7 +39,10 @@ describe Gitlab::Shell, lib: true do end it 'creates and links the secret token file' do + secret_token = Gitlab::Shell.secret_token + expect(File.exist?(secret_file)).to be(true) + expect(File.read(secret_file).chomp).to eq(secret_token) expect(File.symlink?(link_file)).to be(true) expect(File.readlink(link_file)).to eq(secret_file) end diff --git a/spec/lib/gitlab/data_builder/push_spec.rb b/spec/lib/gitlab/data_builder/push_spec.rb index b73434e8dd7..a379f798a16 100644 --- a/spec/lib/gitlab/data_builder/push_spec.rb +++ b/spec/lib/gitlab/data_builder/push_spec.rb @@ -8,13 +8,13 @@ describe Gitlab::DataBuilder::Push, lib: true do let(:data) { described_class.build_sample(project, user) } it { expect(data).to be_a(Hash) } - it { expect(data[:before]).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } - it { expect(data[:after]).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + it { expect(data[:before]).to eq('1b12f15a11fc6e62177bef08f47bc7b5ce50b141') } + it { expect(data[:after]).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') } it { expect(data[:ref]).to eq('refs/heads/master') } it { expect(data[:commits].size).to eq(3) } it { expect(data[:total_commits_count]).to eq(3) } - it { expect(data[:commits].first[:added]).to eq(['gitlab-grack']) } - it { expect(data[:commits].first[:modified]).to eq(['.gitmodules']) } + it { expect(data[:commits].first[:added]).to eq(['bar/branch-test.txt']) } + it { expect(data[:commits].first[:modified]).to eq([]) } it { expect(data[:commits].first[:removed]).to eq([]) } include_examples 'project hook data with deprecateds' diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 006569254a6..5d5836e9bee 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -118,7 +118,7 @@ project: - creator - group - namespace -- board +- boards - last_event - services - campfire_service diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb index 2e19d590d83..ea65a5dfed1 100644 --- a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb @@ -26,10 +26,11 @@ describe 'Import/Export attribute configuration', lib: true do it 'has no new columns' do relation_names.each do |relation_name| relation_class = relation_class_for_name(relation_name) + relation_attributes = relation_class.new.attributes.keys 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) + current_attributes = parsed_attributes(relation_name, relation_attributes) safe_attributes = safe_model_attributes[relation_class.to_s] new_attributes = current_attributes - safe_attributes diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 0e4130e8a3a..c8207e58e90 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -628,7 +628,7 @@ describe Notify do it_behaves_like 'a user cannot unsubscribe through footer link' it 'has the correct subject' do - is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/ + is_expected.to have_subject /Re: #{project.name} | #{commit.title} \(#{commit.short_id}\)/ end it 'contains a link to the commit' do diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 305f8bc88cc..c4486a32082 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -9,6 +9,10 @@ RSpec.describe AbuseReport, type: :model do describe 'associations' do it { is_expected.to belong_to(:reporter).class_name('User') } it { is_expected.to belong_to(:user) } + + it "aliases reporter to author" do + expect(subject.author).to be(subject.reporter) + end end describe 'validations' do diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index c5658bd26e1..0b72a2f979b 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe Appearance, type: :model do - subject { create(:appearance) } + subject { build(:appearance) } it { is_expected.to be_valid } diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index d3e6a6648cc..51be3f36135 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -164,10 +164,10 @@ eos let(:data) { commit.hook_attrs(with_changed_files: true) } it { expect(data).to be_a(Hash) } - it { expect(data[:message]).to include('Add submodule from gitlab.com') } - it { expect(data[:timestamp]).to eq('2014-02-27T11:01:38+02:00') } - it { expect(data[:added]).to eq(["gitlab-grack"]) } - it { expect(data[:modified]).to eq([".gitmodules"]) } + it { expect(data[:message]).to include('adds bar folder and branch-test text file to check Repository merged_to_root_ref method') } + it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46+00:00') } + it { expect(data[:added]).to eq(["bar/branch-test.txt"]) } + it { expect(data[:modified]).to eq([]) } it { expect(data[:removed]).to eq([]) } end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb new file mode 100644 index 00000000000..15cd3a7ed70 --- /dev/null +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -0,0 +1,181 @@ +require 'spec_helper' + +describe CacheMarkdownField do + CacheMarkdownField::CACHING_CLASSES << "ThingWithMarkdownFields" + + # The minimum necessary ActiveModel to test this concern + class ThingWithMarkdownFields + include ActiveModel::Model + include ActiveModel::Dirty + + include ActiveModel::Serialization + + class_attribute :attribute_names + self.attribute_names = [] + + def attributes + attribute_names.each_with_object({}) do |name, hsh| + hsh[name.to_s] = send(name) + end + end + + extend ActiveModel::Callbacks + define_model_callbacks :save + + include CacheMarkdownField + cache_markdown_field :foo + cache_markdown_field :baz, pipeline: :single_line + + def self.add_attr(attr_name) + self.attribute_names += [attr_name] + define_attribute_methods(attr_name) + attr_reader(attr_name) + define_method("#{attr_name}=") do |val| + send("#{attr_name}_will_change!") unless val == send(attr_name) + instance_variable_set("@#{attr_name}", val) + end + end + + [:foo, :foo_html, :bar, :baz, :baz_html].each do |attr_name| + add_attr(attr_name) + end + + def initialize(*) + super + + # Pretend new is load + clear_changes_information + end + + def save + run_callbacks :save do + changes_applied + end + end + end + + CacheMarkdownField::CACHING_CLASSES.delete("ThingWithMarkdownFields") + + def thing_subclass(new_attr) + Class.new(ThingWithMarkdownFields) { add_attr(new_attr) } + end + + let(:markdown) { "`Foo`" } + let(:html) { "<p><code>Foo</code></p>" } + + let(:updated_markdown) { "`Bar`" } + let(:updated_html) { "<p><code>Bar</code></p>" } + + subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) } + + describe ".attributes" do + it "excludes cache attributes" do + expect(thing_subclass(:qux).new.attributes.keys.sort).to eq(%w[bar baz foo qux]) + end + end + + describe ".cache_markdown_field" do + it "refuses to allow untracked classes" do + expect { thing_subclass(:qux).__send__(:cache_markdown_field, :qux) }.to raise_error(RuntimeError) + end + end + + context "an unchanged markdown field" do + before do + subject.foo = subject.foo + subject.save + end + + it { expect(subject.foo).to eq(markdown) } + it { expect(subject.foo_html).to eq(html) } + it { expect(subject.foo_html_changed?).not_to be_truthy } + end + + context "a changed markdown field" do + before do + subject.foo = updated_markdown + subject.save + end + + it { expect(subject.foo_html).to eq(updated_html) } + end + + context "a non-markdown field changed" do + before do + subject.bar = "OK" + subject.save + end + + it { expect(subject.bar).to eq("OK") } + it { expect(subject.foo).to eq(markdown) } + it { expect(subject.foo_html).to eq(html) } + end + + describe '#banzai_render_context' do + it "sets project to nil if the object lacks a project" do + context = subject.banzai_render_context(:foo) + expect(context).to have_key(:project) + expect(context[:project]).to be_nil + end + + it "excludes author if the object lacks an author" do + context = subject.banzai_render_context(:foo) + expect(context).not_to have_key(:author) + end + + it "raises if the context for an unrecognised field is requested" do + expect{subject.banzai_render_context(:not_found)}.to raise_error(ArgumentError) + end + + it "includes the pipeline" do + context = subject.banzai_render_context(:baz) + expect(context[:pipeline]).to eq(:single_line) + end + + it "returns copies of the context template" do + template = subject.cached_markdown_fields[:baz] + copy = subject.banzai_render_context(:baz) + expect(copy).not_to be(template) + end + + context "with a project" do + subject { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project) } + + it "sets the project in the context" do + context = subject.banzai_render_context(:foo) + expect(context).to have_key(:project) + expect(context[:project]).to eq(:project) + end + + it "invalidates the cache when project changes" do + subject.project = :new_project + allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) + + subject.save + + expect(subject.foo_html).to eq(updated_html) + expect(subject.baz_html).to eq(updated_html) + end + end + + context "with an author" do + subject { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author) } + + it "sets the author in the context" do + context = subject.banzai_render_context(:foo) + expect(context).to have_key(:author) + expect(context[:author]).to eq(:author) + end + + it "invalidates the cache when author changes" do + subject.author = :new_author + allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) + + subject.save + + expect(subject.foo_html).to eq(updated_html) + expect(subject.baz_html).to eq(updated_html) + end + end + end +end diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index b9381e33914..7691d690db0 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/spec/models/cycle_analytics/code_spec.rb @@ -8,35 +8,69 @@ describe 'CycleAnalytics#code', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec( - phase: :code, - data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, - start_time_conditions: [["issue mentioned in a commit", - -> (context, data) do - context.create_commit_referencing_issue(data[:issue]) - end]], - end_time_conditions: [["merge request that closes issue is created", - -> (context, data) do - context.create_merge_request_closing_issue(data[:issue]) - end]], - post_fn: -> (context, data) do - context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master - end) - - context "when a regular merge request (that doesn't close the issue) is created" do - it "returns nil" do - 5.times do - issue = create(:issue, project: project) - - create_commit_referencing_issue(issue) - create_merge_request_closing_issue(issue, message: "Closes nothing") - - merge_merge_requests_closing_issue(issue) - deploy_master + context 'with deployment' do + generate_cycle_analytics_spec( + phase: :code, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["issue mentioned in a commit", + -> (context, data) do + context.create_commit_referencing_issue(data[:issue]) + end]], + end_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) + + context "when a regular merge request (that doesn't close the issue) is created" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + + create_commit_referencing_issue(issue) + create_merge_request_closing_issue(issue, message: "Closes nothing") + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.code).to be_nil end + end + end - expect(subject.code).to be_nil + context 'without deployment' do + generate_cycle_analytics_spec( + phase: :code, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["issue mentioned in a commit", + -> (context, data) do + context.create_commit_referencing_issue(data[:issue]) + end]], + end_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + end) + + context "when a regular merge request (that doesn't close the issue) is created" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + + create_commit_referencing_issue(issue) + create_merge_request_closing_issue(issue, message: "Closes nothing") + + merge_merge_requests_closing_issue(issue) + end + + expect(subject.code).to be_nil + end end end end diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index e9cc71254ab..f649b44d367 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -28,7 +28,6 @@ describe 'CycleAnalytics#issue', models: true do if data[:issue].persisted? context.create_merge_request_closing_issue(data[:issue].reload) context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master end end) @@ -41,7 +40,6 @@ describe 'CycleAnalytics#issue', models: true do create_merge_request_closing_issue(issue) merge_merge_requests_closing_issue(issue) - deploy_master end expect(subject.issue).to be_nil diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index 5b8c96dc992..2cdefbeef21 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -31,7 +31,6 @@ describe 'CycleAnalytics#plan', feature: true do post_fn: -> (context, data) do context.create_merge_request_closing_issue(data[:issue], source_branch: data[:branch_name]) context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master end) context "when a regular label (instead of a list label) is added to the issue" do @@ -44,7 +43,6 @@ describe 'CycleAnalytics#plan', feature: true do create_merge_request_closing_issue(issue, source_branch: branch_name) merge_merge_requests_closing_issue(issue) - deploy_master expect(subject.issue).to be_nil end diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index b6e26d8f261..0ed080a42b1 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/spec/models/cycle_analytics/review_spec.rb @@ -19,14 +19,12 @@ describe 'CycleAnalytics#review', feature: true do -> (context, data) do context.merge_merge_requests_closing_issue(data[:issue]) end]], - post_fn: -> (context, data) { context.deploy_master }) + post_fn: nil) context "when a regular merge request (that doesn't close the issue) is created and merged" do it "returns nil" do 5.times do MergeRequests::MergeService.new(project, user).execute(create(:merge_request)) - - deploy_master end expect(subject.review).to be_nil diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index 89ace0b2742..02ddfeed9c1 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/spec/models/cycle_analytics/test_spec.rb @@ -20,7 +20,6 @@ describe 'CycleAnalytics#test', feature: true do end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]], post_fn: -> (context, data) do context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master end) context "when the pipeline is for a regular merge request (that doesn't close an issue)" do @@ -34,7 +33,6 @@ describe 'CycleAnalytics#test', feature: true do pipeline.succeed! merge_merge_requests_closing_issue(issue) - deploy_master end expect(subject.test).to be_nil @@ -48,8 +46,6 @@ describe 'CycleAnalytics#test', feature: true do pipeline.run! pipeline.succeed! - - deploy_master end expect(subject.test).to be_nil @@ -67,7 +63,6 @@ describe 'CycleAnalytics#test', feature: true do pipeline.drop! merge_merge_requests_closing_issue(issue) - deploy_master end expect(subject.test).to be_nil @@ -85,7 +80,6 @@ describe 'CycleAnalytics#test', feature: true do pipeline.cancel! merge_merge_requests_closing_issue(issue) - deploy_master end expect(subject.test).to be_nil diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index 6a90598a629..93623e8e99b 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -1,9 +1,6 @@ require 'spec_helper' describe DeployKey, models: true do - let(:project) { create(:project) } - let(:deploy_key) { create(:deploy_key, projects: [project]) } - describe "Associations" do it { is_expected.to have_many(:deploy_keys_projects) } it { is_expected.to have_many(:projects) } diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index bfff639ad78..01a4a53a264 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -38,5 +38,14 @@ describe Deployment, models: true do expect(deployment.includes_commit?(commit)).to be true end end + + context 'when the SHA for the deployment does not exist in the repo' do + it 'returns false' do + deployment.update(sha: Gitlab::Git::BLANK_SHA) + commit = project.commit + + expect(deployment.includes_commit?(commit)).to be false + end + end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6b1867a44e1..e172ee8e590 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -64,6 +64,23 @@ describe Environment, models: true do end end + describe '#first_deployment_for' do + let(:project) { create(:project) } + let!(:environment) { create(:environment, project: project) } + let!(:deployment) { create(:deployment, environment: environment, ref: commit.parent.id) } + let!(:deployment1) { create(:deployment, environment: environment, ref: commit.id) } + let(:head_commit) { project.commit } + let(:commit) { project.commit.parent } + + it 'returns deployment id for the environment' do + expect(environment.first_deployment_for(commit)).to eq deployment1 + end + + it 'return nil when no deployment is found' do + expect(environment.first_deployment_for(head_commit)).to eq nil + end + end + describe '#environment_type' do subject { environment.environment_type } diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index af5002487cc..733b79079ed 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -27,17 +27,17 @@ describe Event, models: true do end describe "Push event" do - before do - project = create(:project) - @user = project.owner - @event = create_event(project, @user) + let(:project) { create(:project) } + let(:user) { project.owner } + let(:event) { create_event(project, user) } + + it do + expect(event.push?).to be_truthy + expect(event.visible_to_user?).to be_truthy + expect(event.tag?).to be_falsey + expect(event.branch_name).to eq("master") + expect(event.author).to eq(user) end - - it { expect(@event.push?).to be_truthy } - it { expect(@event.visible_to_user?).to be_truthy } - it { expect(@event.tag?).to be_falsey } - it { expect(@event.branch_name).to eq("master") } - it { expect(@event.author).to eq(@user) } end describe '#note?' do @@ -59,8 +59,8 @@ describe Event, models: true do describe '#visible_to_user?' do let(:project) { create(:empty_project, :public) } let(:non_member) { create(:user) } - let(:member) { create(:user) } - let(:guest) { create(:user) } + let(:member) { create(:user) } + let(:guest) { create(:user) } let(:author) { create(:author) } let(:assignee) { create(:user) } let(:admin) { create(:admin) } @@ -79,23 +79,27 @@ describe Event, models: true do context 'for non confidential issues' do let(:target) { issue } - it { expect(event.visible_to_user?(non_member)).to eq true } - it { expect(event.visible_to_user?(author)).to eq true } - it { expect(event.visible_to_user?(assignee)).to eq true } - it { expect(event.visible_to_user?(member)).to eq true } - it { expect(event.visible_to_user?(guest)).to eq true } - it { expect(event.visible_to_user?(admin)).to eq true } + it do + expect(event.visible_to_user?(non_member)).to eq true + expect(event.visible_to_user?(author)).to eq true + expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq true + expect(event.visible_to_user?(admin)).to eq true + end end context 'for confidential issues' do let(:target) { confidential_issue } - it { expect(event.visible_to_user?(non_member)).to eq false } - it { expect(event.visible_to_user?(author)).to eq true } - it { expect(event.visible_to_user?(assignee)).to eq true } - it { expect(event.visible_to_user?(member)).to eq true } - it { expect(event.visible_to_user?(guest)).to eq false } - it { expect(event.visible_to_user?(admin)).to eq true } + it do + expect(event.visible_to_user?(non_member)).to eq false + expect(event.visible_to_user?(author)).to eq true + expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq false + expect(event.visible_to_user?(admin)).to eq true + end end end @@ -103,23 +107,27 @@ describe Event, models: true do context 'on non confidential issues' do let(:target) { note_on_issue } - it { expect(event.visible_to_user?(non_member)).to eq true } - it { expect(event.visible_to_user?(author)).to eq true } - it { expect(event.visible_to_user?(assignee)).to eq true } - it { expect(event.visible_to_user?(member)).to eq true } - it { expect(event.visible_to_user?(guest)).to eq true } - it { expect(event.visible_to_user?(admin)).to eq true } + it do + expect(event.visible_to_user?(non_member)).to eq true + expect(event.visible_to_user?(author)).to eq true + expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq true + expect(event.visible_to_user?(admin)).to eq true + end end context 'on confidential issues' do let(:target) { note_on_confidential_issue } - it { expect(event.visible_to_user?(non_member)).to eq false } - it { expect(event.visible_to_user?(author)).to eq true } - it { expect(event.visible_to_user?(assignee)).to eq true } - it { expect(event.visible_to_user?(member)).to eq true } - it { expect(event.visible_to_user?(guest)).to eq false } - it { expect(event.visible_to_user?(admin)).to eq true } + it do + expect(event.visible_to_user?(non_member)).to eq false + expect(event.visible_to_user?(author)).to eq true + expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq false + expect(event.visible_to_user?(admin)).to eq true + end end end @@ -129,12 +137,27 @@ describe Event, models: true do let(:note_on_merge_request) { create(:legacy_diff_note_on_merge_request, noteable: merge_request, project: project) } let(:target) { note_on_merge_request } - it { expect(event.visible_to_user?(non_member)).to eq true } - it { expect(event.visible_to_user?(author)).to eq true } - it { expect(event.visible_to_user?(assignee)).to eq true } - it { expect(event.visible_to_user?(member)).to eq true } - it { expect(event.visible_to_user?(guest)).to eq true } - it { expect(event.visible_to_user?(admin)).to eq true } + it do + expect(event.visible_to_user?(non_member)).to eq true + expect(event.visible_to_user?(author)).to eq true + expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq true + expect(event.visible_to_user?(admin)).to eq true + end + + context 'private project' do + let(:project) { create(:project, :private) } + + it do + expect(event.visible_to_user?(non_member)).to eq false + expect(event.visible_to_user?(author)).to eq true + expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq false + expect(event.visible_to_user?(admin)).to eq true + end + end end end @@ -203,6 +226,6 @@ describe Event, models: true do action: Event::PUSHED, data: data, author_id: user.id - }.merge(attrs)) + }.merge!(attrs)) end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index fd4a2beff58..7fc6ed1dd54 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -5,9 +5,6 @@ describe Key, models: true do it { is_expected.to belong_to(:user) } end - describe "Mass assignment" do - end - describe "Validation" do it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:key) } diff --git a/spec/models/label_link_spec.rb b/spec/models/label_link_spec.rb index 5e6f8ca1528..c18ed8574b1 100644 --- a/spec/models/label_link_spec.rb +++ b/spec/models/label_link_spec.rb @@ -1,8 +1,7 @@ require 'spec_helper' describe LabelLink, models: true do - let(:label) { create(:label_link) } - it { expect(label).to be_valid } + it { expect(build(:label_link)).to be_valid } it { is_expected.to belong_to(:label) } it { is_expected.to belong_to(:target) } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 530a7def553..e5007424041 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -6,9 +6,9 @@ describe MergeRequestDiff, models: true do it { expect(subject).to be_valid } it { expect(subject).to be_persisted } - it { expect(subject.commits.count).to eq(5) } - it { expect(subject.diffs.count).to eq(8) } - it { expect(subject.head_commit_sha).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + it { expect(subject.commits.count).to eq(29) } + it { expect(subject.diffs.count).to eq(20) } + it { expect(subject.head_commit_sha).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') } it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } end @@ -44,6 +44,16 @@ describe MergeRequestDiff, models: true do end end + context 'when the raw diffs have invalid content' do + before { mr_diff.update_attributes(st_diffs: ["--broken-diff"]) } + + it 'returns an empty DiffCollection' do + expect(mr_diff.raw_diffs.to_a).to be_empty + expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection) + expect(mr_diff.raw_diffs).to be_empty + end + end + context 'when the raw diffs exist' do it 'returns the diffs' do expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection) @@ -64,27 +74,43 @@ describe MergeRequestDiff, models: true do end 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 + 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 + 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' + context 'when commits were loaded' do + before do + subject.commits end - context 'when commits were not loaded' do - it_behaves_like 'returning all commits SHA' - 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 + + describe '#compare_with' do + subject { create(:merge_request, source_branch: 'fix').merge_request_diff } + + it 'delegates compare to the service' do + expect(CompareService).to receive(:new).and_call_original + + subject.compare_with(nil) + end + + it 'uses git diff A..B approach by default' do + diffs = subject.compare_with('0b4bc9a49b562e85de7cc9e834518ea6828729b9').diffs + + expect(diffs.size).to eq(3) end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 38b6da50168..5884b4cff8c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -489,7 +489,7 @@ describe MergeRequest, models: true do subject(:merge_request_with_divergence) { create(:merge_request, :diverged, source_project: project, target_project: project) } it 'counts commits that are on target branch but not on source branch' do - expect(subject.diverged_commits_count).to eq(5) + expect(subject.diverged_commits_count).to eq(29) end end @@ -497,7 +497,7 @@ describe MergeRequest, models: true do subject(:merge_request_fork_with_divergence) { create(:merge_request, :diverged, source_project: fork_project, target_project: project) } it 'counts commits that are on target branch but not on source branch' do - expect(subject.diverged_commits_count).to eq(5) + expect(subject.diverged_commits_count).to eq(29) end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 544920d1824..431b3e4435f 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -114,6 +114,7 @@ describe Namespace, models: true do it "cleans the path and makes sure it's available" do expect(Namespace.clean_path("-john+gitlab-ETC%.git@gmail.com")).to eq("johngitlab-ETC2") + expect(Namespace.clean_path("--%+--valid_*&%name=.git.%.atom.atom.@email.com")).to eq("valid_name") end end end diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index 2fa6715fcaf..c5ff1941378 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -11,7 +11,7 @@ describe ProjectGroupLink do it { should validate_presence_of(:project_id) } it { should validate_uniqueness_of(:group_id).scoped_to(:project_id).with_message(/already shared/) } - it { should validate_presence_of(:group_id) } + it { should validate_presence_of(:group) } it { should validate_presence_of(:group_access) } end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e52d4aaf884..67dbcc362f6 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -24,7 +24,7 @@ describe Project, models: true do it { is_expected.to have_one(:slack_service).dependent(:destroy) } it { is_expected.to have_one(:pushover_service).dependent(:destroy) } it { is_expected.to have_one(:asana_service).dependent(:destroy) } - it { is_expected.to have_one(:board).dependent(:destroy) } + it { is_expected.to have_many(:boards).dependent(:destroy) } it { is_expected.to have_one(:campfire_service).dependent(:destroy) } it { is_expected.to have_one(:drone_ci_service).dependent(:destroy) } it { is_expected.to have_one(:emails_on_push_service).dependent(:destroy) } @@ -94,6 +94,15 @@ describe Project, models: true do end end end + + describe '#boards' do + it 'raises an error when attempting to add more than one board to the project' do + subject.boards.build + + expect { subject.boards.build }.to raise_error(Project::BoardLimitExceeded, 'Number of permitted boards exceeded') + expect(subject.boards.size).to eq 1 + end + end end describe 'modules' do @@ -219,7 +228,6 @@ describe Project, models: true do describe 'Respond to' do it { is_expected.to respond_to(:url_to_repo) } it { is_expected.to respond_to(:repo_exists?) } - it { is_expected.to respond_to(:update_merge_requests) } it { is_expected.to respond_to(:execute_hooks) } it { is_expected.to respond_to(:owner) } it { is_expected.to respond_to(:path_with_namespace) } @@ -308,7 +316,9 @@ describe Project, models: true do end describe 'last_activity methods' do - let(:project) { create(:project, last_activity_at: 2.hours.ago) } + let(:timestamp) { 2.hours.ago } + # last_activity_at gets set to created_at upon creation + let(:project) { create(:project, created_at: timestamp, updated_at: timestamp) } describe 'last_activity' do it 'alias last_activity to last_event' do @@ -322,6 +332,7 @@ describe Project, models: true do it 'returns the creation date of the project\'s last event if present' do new_event = create(:event, project: project, created_at: Time.now) + project.reload expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) end @@ -377,26 +388,6 @@ describe Project, models: true do end end - describe '#update_merge_requests' do - let(:project) { create(:project) } - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:key) { create(:key, user_id: project.owner.id) } - let(:prev_commit_id) { merge_request.commits.last.id } - let(:commit_id) { merge_request.commits.first.id } - - it 'closes merge request if last commit from source branch was pushed to target branch' do - project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.target_branch}", key.user) - merge_request.reload - expect(merge_request.merged?).to be_truthy - end - - it 'updates merge request commits with new one if pushed to source branch' do - project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user) - merge_request.reload - expect(merge_request.diff_head_sha).to eq(commit_id) - end - end - describe '.find_with_namespace' do context 'with namespace' do before do @@ -518,7 +509,7 @@ describe Project, models: true do end describe '#cache_has_external_issue_tracker' do - let(:project) { create(:project) } + let(:project) { create(:project, has_external_issue_tracker: nil) } it 'stores true if there is any external_issue_tracker' do services = double(:service, external_issue_trackers: [RedmineService.new]) @@ -797,32 +788,14 @@ describe Project, models: true do end create(:note_on_commit, project: project2) - end - - describe 'without an explicit start date' do - subject { described_class.trending.to_a } - it 'sorts Projects by the amount of notes in descending order' do - expect(subject).to eq([project1, project2]) - end + TrendingProject.refresh! end - describe 'with an explicit start date' do - let(:date) { 2.months.ago } - - subject { described_class.trending(date).to_a } + subject { described_class.trending.to_a } - before do - 2.times do - # Little fix for special issue related to Fractional Seconds support for MySQL. - # See: https://github.com/rails/rails/pull/14359/files - create(:note_on_commit, project: project2, created_at: date + 1) - end - end - - it 'sorts Projects by the amount of notes in descending order' do - expect(subject).to eq([project2, project1]) - end + it 'sorts projects by the amount of notes in descending order' do + expect(subject).to eq([project1, project2]) end it 'does not take system notes into account' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 98c64c079b9..f977cf73673 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -7,15 +7,18 @@ describe Repository, models: true do let(:project) { create(:project) } let(:repository) { project.repository } let(:user) { create(:user) } + let(:commit_options) do author = repository.user_to_committer(user) { message: 'Test message', committer: author, author: author } end + let(:merge_commit) do merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) merge_commit_id = repository.merge(user, merge_request, commit_options) repository.commit(merge_commit_id) end + let(:author_email) { FFaker::Internet.email } # I have to remove periods from the end of the name @@ -90,6 +93,26 @@ describe Repository, models: true do end end + describe '#ref_name_for_sha' do + context 'ref found' do + it 'returns the ref' do + allow_any_instance_of(Gitlab::Popen).to receive(:popen). + and_return(["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]) + + expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq 'refs/environments/production/77' + end + end + + context 'ref not found' do + it 'returns nil' do + allow_any_instance_of(Gitlab::Popen).to receive(:popen). + and_return(["", 0]) + + expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq nil + end + end + end + describe '#last_commit_for_path' do subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } @@ -97,12 +120,20 @@ describe Repository, models: true do end describe '#find_commits_by_message' do - subject { repository.find_commits_by_message('submodule').map{ |k| k.id } } + it 'returns commits with messages containing a given string' do + commit_ids = repository.find_commits_by_message('submodule').map(&:id) + + expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + expect(commit_ids).to include('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') + expect(commit_ids).to include('cfe32cf61b73a0d5e9f13e774abde7ff789b1660') + expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') + end - it { is_expected.to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } - it { is_expected.to include('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } - it { is_expected.to include('cfe32cf61b73a0d5e9f13e774abde7ff789b1660') } - it { is_expected.not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') } + it 'is case insensitive' do + commit_ids = repository.find_commits_by_message('SUBMODULE').map(&:id) + + expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + end end describe '#blob_at' do @@ -114,11 +145,30 @@ describe Repository, models: true do end describe '#merged_to_root_ref?' do - context 'merged branch' do + context 'merged branch without ff' do + subject { repository.merged_to_root_ref?('branch-merged') } + + it { is_expected.to be_truthy } + end + + # If the HEAD was ff then it will be false + context 'merged with ff' do subject { repository.merged_to_root_ref?('improve/awesome') } it { is_expected.to be_truthy } end + + context 'not merged branch' do + subject { repository.merged_to_root_ref?('not-merged-branch') } + + it { is_expected.to be_falsey } + end + + context 'default branch' do + subject { repository.merged_to_root_ref?('master') } + + it { is_expected.to be_falsey } + end end describe '#can_be_merged?' do @@ -316,7 +366,7 @@ describe Repository, models: true do subject { results.first } it { is_expected.to be_an String } - it { expect(subject.lines[2]).to eq("master:CHANGELOG:188: - Feature: Replace teams with group membership\n") } + it { expect(subject.lines[2]).to eq("master:CHANGELOG:190: - Feature: Replace teams with group membership\n") } end end @@ -960,10 +1010,10 @@ describe Repository, models: true do context 'cherry-picking a merge commit' do it 'cherry-picks the changes' do - expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).to be_nil + expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil - repository.cherry_pick(user, pickable_merge, 'master') - expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).not_to be_nil + repository.cherry_pick(user, pickable_merge, 'improve/awesome') + expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil end end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index ed1bc9271ae..43937a54b2c 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -238,7 +238,7 @@ describe Service, models: true do it "updates the has_external_issue_tracker boolean" do expect do service.save! - end.to change { service.project.has_external_issue_tracker }.from(nil).to(true) + end.to change { service.project.has_external_issue_tracker }.from(false).to(true) end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index e6bc5296398..f62f6bacbaa 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -46,6 +46,13 @@ describe Snippet, models: true do end end + describe "#content_html_invalidated?" do + let(:snippet) { create(:snippet, content: "md", content_html: "html", file_name: "foo.md") } + it "invalidates the HTML cache of content when the filename changes" do + expect { snippet.file_name = "foo.rb" }.to change { snippet.content_html_invalidated? }.from(false).to(true) + end + end + describe '.search' do let(:snippet) { create(:snippet) } diff --git a/spec/models/trending_project_spec.rb b/spec/models/trending_project_spec.rb new file mode 100644 index 00000000000..cc28c6d4004 --- /dev/null +++ b/spec/models/trending_project_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe TrendingProject do + let(:user) { create(:user) } + let(:public_project1) { create(:empty_project, :public) } + let(:public_project2) { create(:empty_project, :public) } + let(:public_project3) { create(:empty_project, :public) } + let(:private_project) { create(:empty_project, :private) } + let(:internal_project) { create(:empty_project, :internal) } + + before do + 3.times do + create(:note_on_commit, project: public_project1) + end + + 2.times do + create(:note_on_commit, project: public_project2) + end + + create(:note_on_commit, project: public_project3, created_at: 5.weeks.ago) + create(:note_on_commit, project: private_project) + create(:note_on_commit, project: internal_project) + end + + describe '.refresh!' do + before do + described_class.refresh! + end + + it 'populates the trending projects table' do + expect(described_class.count).to eq(2) + end + + it 'removes existing rows before populating the table' do + described_class.refresh! + + expect(described_class.count).to eq(2) + end + + it 'stores the project IDs for every trending project' do + rows = described_class.order(id: :asc).all + + expect(rows[0].project_id).to eq(public_project1.id) + expect(rows[1].project_id).to eq(public_project2.id) + end + + it 'does not store projects that fall out of the trending time range' do + expect(described_class.where(project_id: public_project3).any?).to eq(false) + end + + it 'stores only public projects' do + expect(described_class.where(project_id: [public_project1.id, public_project2.id]).count).to eq(2) + expect(described_class.where(project_id: [private_project.id, internal_project.id]).count).to eq(0) + end + end +end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index a7a06744428..658e3c13a73 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -1,20 +1,69 @@ require 'spec_helper' describe ProjectPolicy, models: true do - let(:project) { create(:empty_project, :public) } let(:guest) { create(:user) } let(:reporter) { create(:user) } let(:dev) { create(:user) } let(:master) { create(:user) } let(:owner) { create(:user) } - let(:admin) { create(:admin) } + let(:project) { create(:empty_project, :public, namespace: owner.namespace) } - let(:users_ordered_by_permissions) do - [nil, guest, reporter, dev, master, owner, admin] + let(:guest_permissions) do + [ + :read_project, :read_board, :read_list, :read_wiki, :read_issue, :read_label, + :read_milestone, :read_project_snippet, :read_project_member, + :read_note, :create_project, :create_issue, :create_note, + :upload_file + ] end - let(:users_permissions) do - users_ordered_by_permissions.map { |u| Ability.allowed(u, project).size } + let(:reporter_permissions) do + [ + :download_code, :fork_project, :create_project_snippet, :update_issue, + :admin_issue, :admin_label, :admin_list, :read_commit_status, :read_build, + :read_container_image, :read_pipeline, :read_environment, :read_deployment, + :read_merge_request + ] + end + + let(:team_member_reporter_permissions) do + [ + :build_download_code, :build_read_container_image + ] + end + + let(:developer_permissions) do + [ + :admin_merge_request, :update_merge_request, :create_commit_status, + :update_commit_status, :create_build, :update_build, :create_pipeline, + :update_pipeline, :create_merge_request, :create_wiki, :push_code, + :resolve_note, :create_container_image, :update_container_image, + :create_environment, :create_deployment + ] + end + + let(:master_permissions) do + [ + :push_code_to_protected_branches, :update_project_snippet, :update_environment, + :update_deployment, :admin_milestone, :admin_project_snippet, + :admin_project_member, :admin_note, :admin_wiki, :admin_project, + :admin_commit_status, :admin_build, :admin_container_image, + :admin_pipeline, :admin_environment, :admin_deployment + ] + end + + let(:public_permissions) do + [ + :download_code, :fork_project, :read_commit_status, :read_pipeline, + :read_container_image, :build_download_code, :build_read_container_image + ] + end + + let(:owner_permissions) do + [ + :change_namespace, :change_visibility_level, :rename_project, :remove_project, + :archive_project, :remove_fork_project, :destroy_merge_request, :destroy_issue + ] end before do @@ -22,16 +71,6 @@ describe ProjectPolicy, models: true do project.team << [master, :master] project.team << [dev, :developer] project.team << [reporter, :reporter] - - group = create(:group) - project.project_group_links.create( - group: group, - group_access: Gitlab::Access::MASTER) - group.add_owner(owner) - end - - it 'returns increasing permissions for each level' do - expect(users_permissions).to eq(users_permissions.sort.uniq) end it 'does not include the read_issue permission when the issue author is not a member of the private project' do @@ -46,4 +85,81 @@ describe ProjectPolicy, models: true do expect(Ability.allowed?(user, :read_issue, project)).to be_falsy end + + context 'abilities for non-public projects' do + let(:project) { create(:empty_project, namespace: owner.namespace) } + + subject { described_class.abilities(current_user, project).to_set } + + context 'with no user' do + let(:current_user) { nil } + + it { is_expected.to be_empty } + end + + context 'guests' do + let(:current_user) { guest } + + it do + is_expected.to include(*guest_permissions) + is_expected.not_to include(*reporter_permissions) + is_expected.not_to include(*team_member_reporter_permissions) + is_expected.not_to include(*developer_permissions) + is_expected.not_to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'reporter' do + let(:current_user) { reporter } + + it do + is_expected.to include(*guest_permissions) + is_expected.to include(*reporter_permissions) + is_expected.to include(*team_member_reporter_permissions) + is_expected.not_to include(*developer_permissions) + is_expected.not_to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'developer' do + let(:current_user) { dev } + + it do + is_expected.to include(*guest_permissions) + is_expected.to include(*reporter_permissions) + is_expected.to include(*team_member_reporter_permissions) + is_expected.to include(*developer_permissions) + is_expected.not_to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'master' do + let(:current_user) { master } + + it do + is_expected.to include(*guest_permissions) + is_expected.to include(*reporter_permissions) + is_expected.to include(*team_member_reporter_permissions) + is_expected.to include(*developer_permissions) + is_expected.to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'owner' do + let(:current_user) { owner } + + it do + is_expected.to include(*guest_permissions) + is_expected.to include(*reporter_permissions) + is_expected.not_to include(*team_member_reporter_permissions) + is_expected.to include(*developer_permissions) + is_expected.to include(*master_permissions) + is_expected.to include(*owner_permissions) + end + end + end end diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb new file mode 100644 index 00000000000..f4b04445c6c --- /dev/null +++ b/spec/requests/api/boards_spec.rb @@ -0,0 +1,192 @@ +require 'spec_helper' + +describe API::API, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:non_member) { create(:user) } + let(:guest) { create(:user) } + let(:admin) { create(:user, :admin) } + let!(:project) { create(:project, :public, creator_id: user.id, namespace: user.namespace ) } + + let!(:dev_label) do + create(:label, title: 'Development', color: '#FFAABB', project: project) + end + + let!(:test_label) do + create(:label, title: 'Testing', color: '#FFAACC', project: project) + end + + let!(:ux_label) do + create(:label, title: 'UX', color: '#FF0000', project: project) + end + + let!(:dev_list) do + create(:list, label: dev_label, position: 1) + end + + let!(:test_list) do + create(:list, label: test_label, position: 2) + end + + let!(:board) do + create(:board, project: project, lists: [dev_list, test_list]) + end + + before do + project.team << [user, :reporter] + project.team << [guest, :guest] + end + + describe "GET /projects/:id/boards" do + let(:base_url) { "/projects/#{project.id}/boards" } + + context "when unauthenticated" do + it "returns authentication error" do + get api(base_url) + + expect(response).to have_http_status(401) + end + end + + context "when authenticated" do + it "returns the project issue board" do + get api(base_url, user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(board.id) + expect(json_response.first['lists']).to be_an Array + expect(json_response.first['lists'].length).to eq(2) + expect(json_response.first['lists'].last).to have_key('position') + end + end + end + + describe "GET /projects/:id/boards/:board_id/lists" do + let(:base_url) { "/projects/#{project.id}/boards/#{board.id}/lists" } + + it 'returns issue board lists' do + get api(base_url, user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + expect(json_response.first['label']['name']).to eq(dev_label.title) + end + + it 'returns 404 if board not found' do + get api("/projects/#{project.id}/boards/22343/lists", user) + + expect(response).to have_http_status(404) + end + end + + describe "GET /projects/:id/boards/:board_id/lists/:list_id" do + let(:base_url) { "/projects/#{project.id}/boards/#{board.id}/lists" } + + it 'returns a list' do + get api("#{base_url}/#{dev_list.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['id']).to eq(dev_list.id) + expect(json_response['label']['name']).to eq(dev_label.title) + expect(json_response['position']).to eq(1) + end + + it 'returns 404 if list not found' do + get api("#{base_url}/5324", user) + + expect(response).to have_http_status(404) + end + end + + describe "POST /projects/:id/board/lists" do + let(:base_url) { "/projects/#{project.id}/boards/#{board.id}/lists" } + + it 'creates a new issue board list' do + post api(base_url, user), + label_id: ux_label.id + + expect(response).to have_http_status(201) + expect(json_response['label']['name']).to eq(ux_label.title) + expect(json_response['position']).to eq(3) + end + + it 'returns 400 when creating a new list if label_id is invalid' do + post api(base_url, user), + label_id: 23423 + + expect(response).to have_http_status(400) + end + + it "returns 403 for project members with guest role" do + put api("#{base_url}/#{test_list.id}", guest), + position: 1 + + expect(response).to have_http_status(403) + end + end + + describe "PUT /projects/:id/boards/:board_id/lists/:list_id to update only position" do + let(:base_url) { "/projects/#{project.id}/boards/#{board.id}/lists" } + + it "updates a list" do + put api("#{base_url}/#{test_list.id}", user), + position: 1 + + expect(response).to have_http_status(200) + expect(json_response['position']).to eq(1) + end + + it "returns 404 error if list id not found" do + put api("#{base_url}/44444", user), + position: 1 + + expect(response).to have_http_status(404) + end + + it "returns 403 for project members with guest role" do + put api("#{base_url}/#{test_list.id}", guest), + position: 1 + + expect(response).to have_http_status(403) + end + end + + describe "DELETE /projects/:id/board/lists/:list_id" do + let(:base_url) { "/projects/#{project.id}/boards/#{board.id}/lists" } + + it "rejects a non member from deleting a list" do + delete api("#{base_url}/#{dev_list.id}", non_member) + + expect(response).to have_http_status(403) + end + + it "rejects a user with guest role from deleting a list" do + delete api("#{base_url}/#{dev_list.id}", guest) + + expect(response).to have_http_status(403) + end + + it "returns 404 error if list id not found" do + delete api("#{base_url}/44444", user) + + expect(response).to have_http_status(404) + end + + context "when the user is project owner" do + let(:owner) { create(:user) } + let(:project) { create(:project, namespace: owner.namespace) } + + it "deletes the list if an admin requests it" do + delete api("#{base_url}/#{dev_list.id}", owner) + + expect(response).to have_http_status(200) + expect(json_response['position']).to eq(1) + end + end + end +end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 10f772c5b1a..66fa0c0c01f 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -5,7 +5,7 @@ describe API::API, api: true do include ApiHelpers let(:user) { create(:user) } let(:user2) { create(:user) } - let!(:project) { create(:project, creator_id: user.id) } + let!(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let!(:master) { create(:project_member, :master, user: user, project: project) } let!(:guest) { create(:project_member, :guest, user: user2, project: project) } let!(:note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') } @@ -13,7 +13,7 @@ describe API::API, api: true do before { project.team << [user, :reporter] } - describe "GET /projects/:id/repository/commits" do + describe "List repository commits" do context "authorized user" do before { project.team << [user2, :reporter] } @@ -53,7 +53,12 @@ describe API::API, api: true do get api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user) - expect(json_response.size).to eq(commits.size - 1) + if commits.size >= 20 + expect(json_response.size).to eq(20) + else + expect(json_response.size).to eq(commits.size - 1) + end + expect(json_response.first["id"]).to eq(commits.second.id) expect(json_response.second["id"]).to eq(commits.third.id) end @@ -69,7 +74,268 @@ describe API::API, api: true do end end - describe "GET /projects:id/repository/commits/:sha" do + describe "Create a commit with multiple files and actions" do + let!(:url) { "/projects/#{project.id}/repository/commits" } + + it 'returns a 403 unauthorized for user without permissions' do + post api(url, user2) + + expect(response).to have_http_status(403) + end + + it 'returns a 400 bad request if no params are given' do + post api(url, user) + + expect(response).to have_http_status(400) + end + + context :create do + let(:message) { 'Created file' } + let!(:invalid_c_params) do + { + branch_name: 'master', + commit_message: message, + actions: [ + { + action: 'create', + file_path: 'files/ruby/popen.rb', + content: 'puts 8' + } + ] + } + end + let!(:valid_c_params) do + { + branch_name: 'master', + commit_message: message, + actions: [ + { + action: 'create', + file_path: 'foo/bar/baz.txt', + content: 'puts 8' + } + ] + } + end + + it 'a new file in project repo' do + post api(url, user), valid_c_params + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq(message) + end + + it 'returns a 400 bad request if file exists' do + post api(url, user), invalid_c_params + + expect(response).to have_http_status(400) + end + end + + context :delete do + let(:message) { 'Deleted file' } + let!(:invalid_d_params) do + { + branch_name: 'markdown', + commit_message: message, + actions: [ + { + action: 'delete', + file_path: 'doc/api/projects.md' + } + ] + } + end + let!(:valid_d_params) do + { + branch_name: 'markdown', + commit_message: message, + actions: [ + { + action: 'delete', + file_path: 'doc/api/users.md' + } + ] + } + end + + it 'an existing file in project repo' do + post api(url, user), valid_d_params + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq(message) + end + + it 'returns a 400 bad request if file does not exist' do + post api(url, user), invalid_d_params + + expect(response).to have_http_status(400) + end + end + + context :move do + let(:message) { 'Moved file' } + let!(:invalid_m_params) do + { + branch_name: 'feature', + commit_message: message, + actions: [ + { + action: 'move', + file_path: 'CHANGELOG', + previous_path: 'VERSION', + content: '6.7.0.pre' + } + ] + } + end + let!(:valid_m_params) do + { + branch_name: 'feature', + commit_message: message, + actions: [ + { + action: 'move', + file_path: 'VERSION.txt', + previous_path: 'VERSION', + content: '6.7.0.pre' + } + ] + } + end + + it 'an existing file in project repo' do + post api(url, user), valid_m_params + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq(message) + end + + it 'returns a 400 bad request if file does not exist' do + post api(url, user), invalid_m_params + + expect(response).to have_http_status(400) + end + end + + context :update do + let(:message) { 'Updated file' } + let!(:invalid_u_params) do + { + branch_name: 'master', + commit_message: message, + actions: [ + { + action: 'update', + file_path: 'foo/bar.baz', + content: 'puts 8' + } + ] + } + end + let!(:valid_u_params) do + { + branch_name: 'master', + commit_message: message, + actions: [ + { + action: 'update', + file_path: 'files/ruby/popen.rb', + content: 'puts 8' + } + ] + } + end + + it 'an existing file in project repo' do + post api(url, user), valid_u_params + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq(message) + end + + it 'returns a 400 bad request if file does not exist' do + post api(url, user), invalid_u_params + + expect(response).to have_http_status(400) + end + end + + context "multiple operations" do + let(:message) { 'Multiple actions' } + let!(:invalid_mo_params) do + { + branch_name: 'master', + commit_message: message, + actions: [ + { + action: 'create', + file_path: 'files/ruby/popen.rb', + content: 'puts 8' + }, + { + action: 'delete', + file_path: 'doc/api/projects.md' + }, + { + action: 'move', + file_path: 'CHANGELOG', + previous_path: 'VERSION', + content: '6.7.0.pre' + }, + { + action: 'update', + file_path: 'foo/bar.baz', + content: 'puts 8' + } + ] + } + end + let!(:valid_mo_params) do + { + branch_name: 'master', + commit_message: message, + actions: [ + { + action: 'create', + file_path: 'foo/bar/baz.txt', + content: 'puts 8' + }, + { + action: 'delete', + file_path: 'Gemfile.zip' + }, + { + action: 'move', + file_path: 'VERSION.txt', + previous_path: 'VERSION', + content: '6.7.0.pre' + }, + { + action: 'update', + file_path: 'files/ruby/popen.rb', + content: 'puts 8' + } + ] + } + end + + it 'are commited as one in project repo' do + post api(url, user), valid_mo_params + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq(message) + end + + it 'return a 400 bad request if there are any issues' do + post api(url, user), invalid_mo_params + + expect(response).to have_http_status(400) + end + end + end + + describe "Get a single commit" do context "authorized user" do it "returns a commit by sha" do get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) @@ -122,7 +388,7 @@ describe API::API, api: true do end end - describe "GET /projects:id/repository/commits/:sha/diff" do + describe "Get the diff of a commit" do context "authorized user" do before { project.team << [user2, :reporter] } @@ -149,7 +415,7 @@ describe API::API, api: true do end end - describe 'GET /projects:id/repository/commits/:sha/comments' do + describe 'Get the comments of a commit' do context 'authorized user' do it 'returns merge_request comments' do get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user) @@ -174,7 +440,7 @@ describe API::API, api: true do end end - describe 'POST /projects:id/repository/commits/:sha/comments' do + describe 'Post comment to commit' do context 'authorized user' do it 'returns comment' do post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment' @@ -186,11 +452,12 @@ describe API::API, api: true do end it 'returns the inline comment' do - post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.raw_diffs.first.new_path, line: 7, line_type: 'new' + post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.raw_diffs.first.new_path, line: 1, line_type: 'new' + expect(response).to have_http_status(201) expect(json_response['note']).to eq('My comment') expect(json_response['path']).to eq(project.repository.commit.raw_diffs.first.new_path) - expect(json_response['line']).to eq(7) + expect(json_response['line']).to eq(1) expect(json_response['line_type']).to eq('new') end diff --git a/spec/requests/api/license_templates_spec.rb b/spec/requests/api/license_templates_spec.rb deleted file mode 100644 index 9a1894d63a2..00000000000 --- a/spec/requests/api/license_templates_spec.rb +++ /dev/null @@ -1,136 +0,0 @@ -require 'spec_helper' - -describe API::API, api: true do - include ApiHelpers - - describe 'Entity' do - before { get api('/licenses/mit') } - - it { expect(json_response['key']).to eq('mit') } - it { expect(json_response['name']).to eq('MIT License') } - it { expect(json_response['nickname']).to be_nil } - it { expect(json_response['popular']).to be true } - it { expect(json_response['html_url']).to eq('http://choosealicense.com/licenses/mit/') } - it { expect(json_response['source_url']).to eq('https://opensource.org/licenses/MIT') } - it { expect(json_response['description']).to include('A permissive license that is short and to the point.') } - it { expect(json_response['conditions']).to eq(%w[include-copyright]) } - it { expect(json_response['permissions']).to eq(%w[commercial-use modifications distribution private-use]) } - it { expect(json_response['limitations']).to eq(%w[no-liability]) } - it { expect(json_response['content']).to include('The MIT License (MIT)') } - end - - describe 'GET /licenses' do - it 'returns a list of available license templates' do - get api('/licenses') - - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.size).to eq(15) - expect(json_response.map { |l| l['key'] }).to include('agpl-3.0') - end - - describe 'the popular parameter' do - context 'with popular=1' do - it 'returns a list of available popular license templates' do - get api('/licenses?popular=1') - - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.size).to eq(3) - expect(json_response.map { |l| l['key'] }).to include('apache-2.0') - end - end - end - end - - describe 'GET /licenses/:key' do - context 'with :project and :fullname given' do - before do - get api("/licenses/#{license_type}?project=My+Awesome+Project&fullname=Anton+#{license_type.upcase}") - end - - context 'for the mit license' do - let(:license_type) { 'mit' } - - it 'returns the license text' do - expect(json_response['content']).to include('The MIT License (MIT)') - end - - it 'replaces placeholder values' do - expect(json_response['content']).to include("Copyright (c) #{Time.now.year} Anton") - end - end - - context 'for the agpl-3.0 license' do - let(:license_type) { 'agpl-3.0' } - - it 'returns the license text' do - expect(json_response['content']).to include('GNU AFFERO GENERAL PUBLIC LICENSE') - end - - it 'replaces placeholder values' do - expect(json_response['content']).to include('My Awesome Project') - expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") - end - end - - context 'for the gpl-3.0 license' do - let(:license_type) { 'gpl-3.0' } - - it 'returns the license text' do - expect(json_response['content']).to include('GNU GENERAL PUBLIC LICENSE') - end - - it 'replaces placeholder values' do - expect(json_response['content']).to include('My Awesome Project') - expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") - end - end - - context 'for the gpl-2.0 license' do - let(:license_type) { 'gpl-2.0' } - - it 'returns the license text' do - expect(json_response['content']).to include('GNU GENERAL PUBLIC LICENSE') - end - - it 'replaces placeholder values' do - expect(json_response['content']).to include('My Awesome Project') - expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") - end - end - - context 'for the apache-2.0 license' do - let(:license_type) { 'apache-2.0' } - - it 'returns the license text' do - expect(json_response['content']).to include('Apache License') - end - - it 'replaces placeholder values' do - expect(json_response['content']).to include("Copyright #{Time.now.year} Anton") - end - end - - context 'for an uknown license' do - let(:license_type) { 'muth-over9000' } - - it 'returns a 404' do - expect(response).to have_http_status(404) - end - end - end - - context 'with no :fullname given' do - context 'with an authenticated user' do - let(:user) { create(:user) } - - it 'replaces the copyright owner placeholder with the name of the current user' do - get api('/licenses/mit', user) - - expect(json_response['content']).to include("Copyright (c) #{Time.now.year} #{user.name}") - end - end - end - end -end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 92032f09b17..d22e0595788 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -97,7 +97,10 @@ describe API::Members, api: true do shared_examples 'POST /:sources/:id/members' do |source_type| context "with :sources == #{source_type.pluralize}" do it_behaves_like 'a 404 response when source is private' do - let(:route) { post api("/#{source_type.pluralize}/#{source.id}/members", stranger) } + let(:route) do + post api("/#{source_type.pluralize}/#{source.id}/members", stranger), + user_id: access_requester.id, access_level: Member::MASTER + end end context 'when authenticated as a non-member or member with insufficient rights' do @@ -105,7 +108,8 @@ describe API::Members, api: true do context "as a #{type}" do it 'returns 403' do user = public_send(type) - post api("/#{source_type.pluralize}/#{source.id}/members", user) + post api("/#{source_type.pluralize}/#{source.id}/members", user), + user_id: access_requester.id, access_level: Member::MASTER expect(response).to have_http_status(403) end @@ -174,7 +178,10 @@ describe API::Members, api: true do shared_examples 'PUT /:sources/:id/members/:user_id' do |source_type| context "with :sources == #{source_type.pluralize}" do it_behaves_like 'a 404 response when source is private' do - let(:route) { put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) } + let(:route) do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger), + access_level: Member::MASTER + end end context 'when authenticated as a non-member or member with insufficient rights' do @@ -182,7 +189,8 @@ describe API::Members, api: true do context "as a #{type}" do it 'returns 403' do user = public_send(type) - put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user), + access_level: Member::MASTER expect(response).to have_http_status(403) end diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 765dc8a8f66..cfcdcad74cd 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -163,9 +163,10 @@ describe API::API, 'ProjectHooks', api: true do expect(response).to have_http_status(404) end - it "returns a 405 error if hook id not given" do + it "returns a 404 error if hook id not given" do delete api("/projects/#{project.id}/hooks", user) - expect(response).to have_http_status(405) + + expect(response).to have_http_status(404) end it "returns a 404 if a user attempts to delete project hooks he/she does not own" do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 4a0d727faea..973928d007a 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -175,6 +175,36 @@ describe API::API, api: true do end end + describe 'GET /projects/visible' do + let(:public_project) { create(:project, :public) } + + before do + public_project + project + project2 + project3 + project4 + end + + it 'returns the projects viewable by the user' do + get api('/projects/visible', user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }). + to contain_exactly(public_project.id, project.id, project2.id, project3.id) + end + + it 'shows only public projects when the user only has access to those' do + get api('/projects/visible', user2) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }). + to contain_exactly(public_project.id) + end + end + describe 'GET /projects/starred' do let(:public_project) { create(:project, :public) } @@ -232,7 +262,7 @@ describe API::API, api: true do post api('/projects', user), project project.each_pair do |k, v| - next if %i{ issues_enabled merge_requests_enabled wiki_enabled }.include?(k) + next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k) expect(json_response[k.to_s]).to eq(v) end @@ -360,7 +390,7 @@ describe API::API, api: true do post api("/projects/user/#{user.id}", admin), project project.each_pair do |k, v| - next if k == :path + next if %i[has_external_issue_tracker path].include?(k) expect(json_response[k.to_s]).to eq(v) end end @@ -558,37 +588,39 @@ describe API::API, api: true do before do note = create(:note_on_issue, note: 'What an awesome day!', project: project) EventCreateService.new.leave_note(note, note.author) - get api("/projects/#{project.id}/events", user) end - it { expect(response).to have_http_status(200) } + it 'returns all events' do + get api("/projects/#{project.id}/events", user) - context 'joined event' do - let(:json_event) { json_response[1] } + expect(response).to have_http_status(200) - it { expect(json_event['action_name']).to eq('joined') } - it { expect(json_event['project_id'].to_i).to eq(project.id) } - it { expect(json_event['author_username']).to eq(user3.username) } - it { expect(json_event['author']['name']).to eq(user3.name) } - end + first_event = json_response.first - context 'comment event' do - let(:json_event) { json_response.first } + expect(first_event['action_name']).to eq('commented on') + expect(first_event['note']['body']).to eq('What an awesome day!') - it { expect(json_event['action_name']).to eq('commented on') } - it { expect(json_event['note']['body']).to eq('What an awesome day!') } + last_event = json_response.last + + expect(last_event['action_name']).to eq('joined') + expect(last_event['project_id'].to_i).to eq(project.id) + expect(last_event['author_username']).to eq(user3.username) + expect(last_event['author']['name']).to eq(user3.name) end end it 'returns a 404 error if not found' do get api('/projects/42/events', user) + expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 Project Not Found') end it 'returns a 404 error if user is not a member' do other_user = create(:user) + get api("/projects/#{project.id}/events", other_user) + expect(response).to have_http_status(404) end end @@ -789,6 +821,20 @@ describe API::API, api: true do expect(response.status).to eq 400 end + it 'returns a 404 error when user cannot read group' do + private_group = create(:group, :private) + + post api("/projects/#{project.id}/share", user), group_id: private_group.id, group_access: Gitlab::Access::DEVELOPER + + expect(response.status).to eq 404 + end + + it 'returns a 404 error when group does not exist' do + post api("/projects/#{project.id}/share", user), group_id: 1234, group_access: Gitlab::Access::DEVELOPER + + expect(response.status).to eq 404 + end + it "returns a 409 error when wrong params passed" do post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: 1234 expect(response.status).to eq 409 diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 80a856a6e90..c4dc2d9006a 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -21,7 +21,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['name']).to eq('encoding') + expect(json_response.first['name']).to eq('bar') expect(json_response.first['type']).to eq('tree') expect(json_response.first['mode']).to eq('040000') end @@ -166,9 +166,9 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response).to be_an Array contributor = json_response.first - expect(contributor['email']).to eq('dmitriy.zaporozhets@gmail.com') - expect(contributor['name']).to eq('Dmitriy Zaporozhets') - expect(contributor['commits']).to eq(13) + expect(contributor['email']).to eq('tiagonbotelho@hotmail.com') + expect(contributor['name']).to eq('tiagonbotelho') + expect(contributor['commits']).to eq(1) expect(contributor['additions']).to eq(0) expect(contributor['deletions']).to eq(0) end diff --git a/spec/requests/api/templates_spec.rb b/spec/requests/api/templates_spec.rb index 5bd5b861792..d32ba60fc4c 100644 --- a/spec/requests/api/templates_spec.rb +++ b/spec/requests/api/templates_spec.rb @@ -3,53 +3,201 @@ require 'spec_helper' describe API::Templates, api: true do include ApiHelpers - context 'global templates' do - describe 'the Template Entity' do - before { get api('/gitignores/Ruby') } + shared_examples_for 'the Template Entity' do |path| + before { get api(path) } - it { expect(json_response['name']).to eq('Ruby') } - it { expect(json_response['content']).to include('*.gem') } + it { expect(json_response['name']).to eq('Ruby') } + it { expect(json_response['content']).to include('*.gem') } + end + + shared_examples_for 'the TemplateList Entity' do |path| + before { get api(path) } + + it { expect(json_response.first['name']).not_to be_nil } + it { expect(json_response.first['content']).to be_nil } + end + + shared_examples_for 'requesting gitignores' do |path| + it 'returns a list of available gitignore templates' do + get api(path) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to be > 15 end + end - describe 'the TemplateList Entity' do - before { get api('/gitignores') } + shared_examples_for 'requesting gitlab-ci-ymls' do |path| + it 'returns a list of available gitlab_ci_ymls' do + get api(path) - it { expect(json_response.first['name']).not_to be_nil } - it { expect(json_response.first['content']).to be_nil } + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).not_to be_nil end + end + + shared_examples_for 'requesting gitlab-ci-yml for Ruby' do |path| + it 'adds a disclaimer on the top' do + get api(path) + + expect(response).to have_http_status(200) + expect(json_response['content']).to start_with("# This file is a template,") + end + end + + shared_examples_for 'the License Template Entity' do |path| + before { get api(path) } + + it 'returns a license template' do + expect(json_response['key']).to eq('mit') + expect(json_response['name']).to eq('MIT License') + expect(json_response['nickname']).to be_nil + expect(json_response['popular']).to be true + expect(json_response['html_url']).to eq('http://choosealicense.com/licenses/mit/') + expect(json_response['source_url']).to eq('https://opensource.org/licenses/MIT') + expect(json_response['description']).to include('A permissive license that is short and to the point.') + expect(json_response['conditions']).to eq(%w[include-copyright]) + expect(json_response['permissions']).to eq(%w[commercial-use modifications distribution private-use]) + expect(json_response['limitations']).to eq(%w[no-liability]) + expect(json_response['content']).to include('The MIT License (MIT)') + end + end - context 'requesting gitignores' do - describe 'GET /gitignores' do - it 'returns a list of available gitignore templates' do - get api('/gitignores') + shared_examples_for 'GET licenses' do |path| + it 'returns a list of available license templates' do + get api(path) - expect(response.status).to eq(200) + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(15) + expect(json_response.map { |l| l['key'] }).to include('agpl-3.0') + end + + describe 'the popular parameter' do + context 'with popular=1' do + it 'returns a list of available popular license templates' do + get api("#{path}?popular=1") + + expect(response).to have_http_status(200) expect(json_response).to be_an Array - expect(json_response.size).to be > 15 + expect(json_response.size).to eq(3) + expect(json_response.map { |l| l['key'] }).to include('apache-2.0') end end end + end - context 'requesting gitlab-ci-ymls' do - describe 'GET /gitlab_ci_ymls' do - it 'returns a list of available gitlab_ci_ymls' do - get api('/gitlab_ci_ymls') + shared_examples_for 'GET licenses/:name' do |path| + context 'with :project and :fullname given' do + before do + get api("#{path}/#{license_type}?project=My+Awesome+Project&fullname=Anton+#{license_type.upcase}") + end - expect(response.status).to eq(200) - expect(json_response).to be_an Array - expect(json_response.first['name']).not_to be_nil + context 'for the mit license' do + let(:license_type) { 'mit' } + + it 'returns the license text' do + expect(json_response['content']).to include('The MIT License (MIT)') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include("Copyright (c) #{Time.now.year} Anton") + end + end + + context 'for the agpl-3.0 license' do + let(:license_type) { 'agpl-3.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('GNU AFFERO GENERAL PUBLIC LICENSE') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include('My Awesome Project') + expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") + end + end + + context 'for the gpl-3.0 license' do + let(:license_type) { 'gpl-3.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('GNU GENERAL PUBLIC LICENSE') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include('My Awesome Project') + expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") + end + end + + context 'for the gpl-2.0 license' do + let(:license_type) { 'gpl-2.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('GNU GENERAL PUBLIC LICENSE') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include('My Awesome Project') + expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") + end + end + + context 'for the apache-2.0 license' do + let(:license_type) { 'apache-2.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('Apache License') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include("Copyright #{Time.now.year} Anton") + end + end + + context 'for an uknown license' do + let(:license_type) { 'muth-over9000' } + + it 'returns a 404' do + expect(response).to have_http_status(404) end end end - describe 'GET /gitlab_ci_ymls/Ruby' do - it 'adds a disclaimer on the top' do - get api('/gitlab_ci_ymls/Ruby') + context 'with no :fullname given' do + context 'with an authenticated user' do + let(:user) { create(:user) } + + it 'replaces the copyright owner placeholder with the name of the current user' do + get api('/templates/licenses/mit', user) - expect(response).to have_http_status(200) - expect(json_response['name']).not_to be_nil - expect(json_response['content']).to start_with("# This file is a template,") + expect(json_response['content']).to include("Copyright (c) #{Time.now.year} #{user.name}") + end end end end + + describe 'with /templates namespace' do + it_behaves_like 'the Template Entity', '/templates/gitignores/Ruby' + it_behaves_like 'the TemplateList Entity', '/templates/gitignores' + it_behaves_like 'requesting gitignores', '/templates/gitignores' + it_behaves_like 'requesting gitlab-ci-ymls', '/templates/gitlab_ci_ymls' + it_behaves_like 'requesting gitlab-ci-yml for Ruby', '/templates/gitlab_ci_ymls/Ruby' + it_behaves_like 'the License Template Entity', '/templates/licenses/mit' + it_behaves_like 'GET licenses', '/templates/licenses' + it_behaves_like 'GET licenses/:name', '/templates/licenses' + end + + describe 'without /templates namespace' do + it_behaves_like 'the Template Entity', '/gitignores/Ruby' + it_behaves_like 'the TemplateList Entity', '/gitignores' + it_behaves_like 'requesting gitignores', '/gitignores' + it_behaves_like 'requesting gitlab-ci-ymls', '/gitlab_ci_ymls' + it_behaves_like 'requesting gitlab-ci-yml for Ruby', '/gitlab_ci_ymls/Ruby' + it_behaves_like 'the License Template Entity', '/licenses/mit' + it_behaves_like 'GET licenses', '/licenses' + it_behaves_like 'GET licenses/:name', '/licenses' + end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index f4ea3bebb4c..f83f4d2c9b1 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -90,8 +90,9 @@ describe API::API, api: true do expect(json_response['message']).to eq('404 Not found') end - it "returns a 404 if invalid ID" do + it "returns a 404 for invalid ID" do get api("/users/1ASDF", user) + expect(response).to have_http_status(404) end end @@ -340,8 +341,10 @@ describe API::API, api: true do expect(json_response['message']).to eq('404 Not found') end - it "raises error for invalid ID" do - expect{put api("/users/ASDF", admin) }.to raise_error(ActionController::RoutingError) + it "returns a 404 if invalid ID" do + put api("/users/ASDF", admin) + + expect(response).to have_http_status(404) end it 'returns 400 error if user does not validate' do @@ -493,8 +496,9 @@ describe API::API, api: true do end.to change{ user.emails.count }.by(1) end - it "raises error for invalid ID" do + it "returns a 400 for invalid ID" do post api("/users/999999/emails", admin) + expect(response).to have_http_status(400) end end @@ -525,9 +529,10 @@ describe API::API, api: true do expect(json_response.first['email']).to eq(email.email) end - it "raises error for invalid ID" do + it "returns a 404 for invalid ID" do put api("/users/ASDF/emails", admin) - expect(response).to have_http_status(405) + + expect(response).to have_http_status(404) end end end @@ -566,8 +571,10 @@ describe API::API, api: true do expect(json_response['message']).to eq('404 Email Not Found') end - it "raises error for invalid ID" do - expect{delete api("/users/ASDF/emails/bar", admin) }.to raise_error(ActionController::RoutingError) + it "returns a 404 for invalid ID" do + delete api("/users/ASDF/emails/bar", admin) + + expect(response).to have_http_status(404) end end end @@ -600,8 +607,10 @@ describe API::API, api: true do expect(json_response['message']).to eq('404 User Not Found') end - it "raises error for invalid ID" do - expect{delete api("/users/ASDF", admin) }.to raise_error(ActionController::RoutingError) + it "returns a 404 for invalid ID" do + delete api("/users/ASDF", admin) + + expect(response).to have_http_status(404) end end @@ -654,6 +663,7 @@ describe API::API, api: true do it "returns 404 Not Found within invalid ID" do get api("/user/keys/42", user) + expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 Not found') end @@ -669,6 +679,7 @@ describe API::API, api: true do it "returns 404 for invalid ID" do get api("/users/keys/ASDF", admin) + expect(response).to have_http_status(404) end end @@ -727,8 +738,10 @@ describe API::API, api: true do expect(response).to have_http_status(401) end - it "raises error for invalid ID" do - expect{delete api("/users/keys/ASDF", admin) }.to raise_error(ActionController::RoutingError) + it "returns a 404 for invalid ID" do + delete api("/users/keys/ASDF", admin) + + expect(response).to have_http_status(404) end end @@ -778,6 +791,7 @@ describe API::API, api: true do it "returns 404 for invalid ID" do get api("/users/emails/ASDF", admin) + expect(response).to have_http_status(404) end end @@ -825,8 +839,10 @@ describe API::API, api: true do expect(response).to have_http_status(401) end - it "raises error for invalid ID" do - expect{delete api("/users/emails/ASDF", admin) }.to raise_error(ActionController::RoutingError) + it "returns a 404 for invalid ID" do + delete api("/users/emails/ASDF", admin) + + expect(response).to have_http_status(404) end end @@ -891,8 +907,64 @@ describe API::API, api: true do expect(json_response['message']).to eq('404 User Not Found') end - it "raises error for invalid ID" do - expect{put api("/users/ASDF/block", admin) }.to raise_error(ActionController::RoutingError) + it "returns a 404 for invalid ID" do + put api("/users/ASDF/block", admin) + + expect(response).to have_http_status(404) + end + end + + describe 'GET /user/:id/events' do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:note) { create(:note_on_issue, note: 'What an awesome day!', project: project) } + + before do + project.add_user(user, :developer) + EventCreateService.new.leave_note(note, user) + end + + context "as a user than cannot see the event's project" do + it 'returns no events' do + other_user = create(:user) + + get api("/users/#{user.id}/events", other_user) + + expect(response).to have_http_status(200) + expect(json_response).to be_empty + end + end + + context "as a user than can see the event's project" do + it_behaves_like 'a paginated resources' do + let(:request) { get api("/users/#{user.id}/events", user) } + end + + context 'joined event' do + it 'returns the "joined" event' do + get api("/users/#{user.id}/events", user) + + comment_event = json_response.find { |e| e['action_name'] == 'commented on' } + + expect(comment_event['project_id'].to_i).to eq(project.id) + expect(comment_event['author_username']).to eq(user.username) + expect(comment_event['note']['id']).to eq(note.id) + expect(comment_event['note']['body']).to eq('What an awesome day!') + + joined_event = json_response.find { |e| e['action_name'] == 'joined' } + + expect(joined_event['project_id'].to_i).to eq(project.id) + expect(joined_event['author_username']).to eq(user.username) + expect(joined_event['author']['name']).to eq(user.name) + end + end + end + + it 'returns a 404 error if not found' do + get api('/users/42/events', user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') end end end diff --git a/spec/requests/api/version_spec.rb b/spec/requests/api/version_spec.rb new file mode 100644 index 00000000000..54b69a0cae7 --- /dev/null +++ b/spec/requests/api/version_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe API::API, api: true do + include ApiHelpers + + describe 'GET /version' do + context 'when unauthenticated' do + it 'returns authentication error' do + get api('/version') + + expect(response).to have_http_status(401) + end + end + + context 'when authenticated' do + let(:user) { create(:user) } + + it 'returns the version information' do + get api('/version', user) + + expect(response).to have_http_status(200) + expect(json_response['version']).to eq(Gitlab::VERSION) + expect(json_response['revision']).to eq(Gitlab::REVISION) + end + end + end +end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index c0c1e62e910..27f0fd22ae6 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -440,8 +440,8 @@ describe 'Git HTTP requests', lib: true 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') + allow_any_instance_of(Repository).to receive(:blob_at).with('b83d6e391c22777fca1ed3012fce84f633d7fed0', 'info/refs') do + Gitlab::Git::Blob.find(project.repository, 'master', 'bar/branch-test.txt') end get "/#{project.path_with_namespace}/blob/master/info/refs" diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 77842057a10..2322430d212 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -337,7 +337,7 @@ describe Projects::CommitsController, 'routing' do end it 'to #show' do - expect(get('/gitlab/gitlabhq/commits/master.atom')).to route_to('projects/commits#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'atom') + expect(get('/gitlab/gitlabhq/commits/master.atom')).to route_to('projects/commits#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master.atom') end end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 4bc3cddd9c2..488dc1a63b0 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -9,7 +9,9 @@ require 'spec_helper' # user_calendar_activities GET /u/:username/calendar_activities(.:format) describe UsersController, "routing" do it "to #show" do - expect(get("/u/User")).to route_to('users#show', username: 'User') + allow(User).to receive(:find_by).and_return(true) + + expect(get("/User")).to route_to('users#show', username: 'User') end it "to #groups" do @@ -264,7 +266,15 @@ describe "Groups", "routing" do end it "also display group#show on the short path" do - expect(get('/1')).to route_to('namespaces#show', id: '1') + allow(Group).to receive(:find_by_path).and_return(true) + + expect(get('/1')).to route_to('groups#show', id: '1') + end + + it "also display group#show with dot in the path" do + allow(Group).to receive(:find_by_path).and_return(true) + + expect(get('/group.with.dot')).to route_to('groups#show', id: 'group.with.dot') end end diff --git a/spec/services/boards/create_service_spec.rb b/spec/services/boards/create_service_spec.rb index a1a4dd4c57c..fde807cc410 100644 --- a/spec/services/boards/create_service_spec.rb +++ b/spec/services/boards/create_service_spec.rb @@ -2,33 +2,31 @@ require 'spec_helper' describe Boards::CreateService, services: true do describe '#execute' do + let(:project) { create(:empty_project) } + subject(:service) { described_class.new(project, double) } context 'when project does not have a board' do - let(:project) { create(:empty_project, board: nil) } - it 'creates a new board' do expect { service.execute }.to change(Board, :count).by(1) end it 'creates default lists' do - service.execute + board = service.execute - expect(project.board.lists.size).to eq 2 - expect(project.board.lists.first).to be_backlog - expect(project.board.lists.last).to be_done + expect(board.lists.size).to eq 2 + expect(board.lists.first).to be_backlog + expect(board.lists.last).to be_done end end context 'when project has a board' do - let!(:project) { create(:project_with_board) } - - it 'does not create a new board' do - expect { service.execute }.not_to change(Board, :count) + before do + create(:board, project: project) end - it 'does not create board lists' do - expect { service.execute }.not_to change(project.board.lists, :count) + it 'does not create a new board' do + expect { service.execute }.not_to change(project.boards, :count) end end end diff --git a/spec/services/boards/issues/create_service_spec.rb b/spec/services/boards/issues/create_service_spec.rb new file mode 100644 index 00000000000..360ee398f77 --- /dev/null +++ b/spec/services/boards/issues/create_service_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Boards::Issues::CreateService, services: true do + describe '#execute' do + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } + let(:user) { create(:user) } + let(:label) { create(:label, project: project, name: 'in-progress') } + let!(:list) { create(:list, board: board, label: label, position: 0) } + + subject(:service) { described_class.new(project, user, board_id: board.id, list_id: list.id, title: 'New issue') } + + before do + project.team << [user, :developer] + end + + it 'delegates the create proceedings to Issues::CreateService' do + expect_any_instance_of(Issues::CreateService).to receive(:execute).once + + service.execute + end + + it 'creates a new issue' do + expect { service.execute }.to change(project.issues, :count).by(1) + end + + it 'adds the label of the list to the issue' do + issue = service.execute + + expect(issue.labels).to eq [label] + end + end +end diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index e65da15aca8..7c206cf3ce7 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Boards::Issues::ListService, services: true do describe '#execute' do let(:user) { create(:user) } - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:bug) { create(:label, project: project, name: 'Bug') } let(:development) { create(:label, project: project, name: 'Development') } @@ -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) { project.board.backlog_list } + let!(:backlog) { create(:backlog_list, board: board) } let!(:list1) { create(:list, board: board, label: development, position: 0) } let!(:list2) { create(:list, board: board, label: testing, position: 1) } - let!(:done) { project.board.done_list } + let!(:done) { create(:done_list, board: board) } let!(:opened_issue1) { create(:labeled_issue, project: project, labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, labels: [p2]) } @@ -30,14 +30,14 @@ describe Boards::Issues::ListService, services: true do let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) } let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3]) } let!(:closed_issue3) { create(:issue, :closed, project: project) } - let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1, development]) } + let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1]) } before do project.team << [user, :developer] end it 'delegates search to IssuesFinder' do - params = { id: list1.id } + params = { board_id: board.id, id: list1.id } expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original @@ -46,7 +46,7 @@ describe Boards::Issues::ListService, services: true do context 'sets default order to priority' do it 'returns opened issues when listing issues from Backlog' do - params = { id: backlog.id } + params = { board_id: board.id, id: backlog.id } issues = described_class.new(project, user, params).execute @@ -54,19 +54,36 @@ describe Boards::Issues::ListService, services: true do end it 'returns closed issues when listing issues from Done' do - params = { id: done.id } + params = { board_id: board.id, id: done.id } issues = described_class.new(project, user, params).execute - expect(issues).to eq [closed_issue2, closed_issue3, closed_issue1] + expect(issues).to eq [closed_issue4, closed_issue2, closed_issue3, closed_issue1] end - it 'returns opened/closed issues that have label list applied when listing issues from a label list' do - params = { id: list1.id } + it 'returns opened issues that have label list applied when listing issues from a label list' do + params = { board_id: board.id, id: list1.id } issues = described_class.new(project, user, params).execute - expect(issues).to eq [closed_issue4, list1_issue3, list1_issue1, list1_issue2] + expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2] + end + end + + context 'with list that does not belong to the board' do + it 'raises an error' do + list = create(:list) + service = described_class.new(project, user, board_id: board.id, id: list.id) + + expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'with invalid list id' do + it 'raises an error' do + service = described_class.new(project, user, board_id: board.id, id: nil) + + expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb index 180f1b08631..c43b2aec490 100644 --- a/spec/services/boards/issues/move_service_spec.rb +++ b/spec/services/boards/issues/move_service_spec.rb @@ -3,17 +3,17 @@ require 'spec_helper' describe Boards::Issues::MoveService, services: true do describe '#execute' do let(:user) { create(:user) } - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board1) { create(:board, project: project) } let(:bug) { create(:label, project: project, name: 'Bug') } let(:development) { create(:label, project: project, name: 'Development') } let(:testing) { create(:label, project: project, name: 'Testing') } - 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) { project.board.done_list } + let!(:backlog) { create(:backlog_list, board: board1) } + let!(:list1) { create(:list, board: board1, label: development, position: 0) } + let!(:list2) { create(:list, board: board1, label: testing, position: 1) } + let!(:done) { create(:done_list, board: board1) } before do project.team << [user, :developer] @@ -22,7 +22,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from backlog' do it 'adds the label of the list it goes to' do issue = create(:labeled_issue, project: project, labels: [bug]) - params = { from_list_id: backlog.id, to_list_id: list1.id } + params = { board_id: board1.id, from_list_id: backlog.id, to_list_id: list1.id } described_class.new(project, user, params).execute(issue) @@ -33,7 +33,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving to backlog' do it 'removes all list-labels' do issue = create(:labeled_issue, project: project, labels: [bug, development, testing]) - params = { from_list_id: list1.id, to_list_id: backlog.id } + params = { board_id: board1.id, from_list_id: list1.id, to_list_id: backlog.id } described_class.new(project, user, params).execute(issue) @@ -44,7 +44,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from backlog to done' do it 'closes the issue' do issue = create(:labeled_issue, project: project, labels: [bug]) - params = { from_list_id: backlog.id, to_list_id: done.id } + params = { board_id: board1.id, from_list_id: backlog.id, to_list_id: done.id } described_class.new(project, user, params).execute(issue) issue.reload @@ -56,7 +56,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving an issue between lists' do let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { from_list_id: list1.id, to_list_id: list2.id } } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } } it 'delegates the label changes to Issues::UpdateService' do expect_any_instance_of(Issues::UpdateService).to receive(:execute).with(issue).once @@ -72,8 +72,12 @@ describe Boards::Issues::MoveService, services: true do end context 'when moving to done' do - let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing]) } - let(:params) { { from_list_id: list2.id, to_list_id: done.id } } + let(:board2) { create(:board, project: project) } + let(:regression) { create(:label, project: project, name: 'Regression') } + let!(:list3) { create(:list, board: board2, label: regression, position: 1) } + + let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression]) } + let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: done.id } } it 'delegates the close proceedings to Issues::CloseService' do expect_any_instance_of(Issues::CloseService).to receive(:execute).with(issue).once @@ -81,7 +85,7 @@ describe Boards::Issues::MoveService, services: true do described_class.new(project, user, params).execute(issue) end - it 'removes all list-labels and close the issue' do + it 'removes all list-labels from project boards and close the issue' do described_class.new(project, user, params).execute(issue) issue.reload @@ -92,7 +96,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from done' do let(:issue) { create(:labeled_issue, :closed, project: project, labels: [bug]) } - let(:params) { { from_list_id: done.id, to_list_id: list2.id } } + let(:params) { { board_id: board1.id, from_list_id: done.id, to_list_id: list2.id } } it 'delegates the re-open proceedings to Issues::ReopenService' do expect_any_instance_of(Issues::ReopenService).to receive(:execute).with(issue).once @@ -112,7 +116,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from done to backlog' do it 'reopens the issue' do issue = create(:labeled_issue, :closed, project: project, labels: [bug]) - params = { from_list_id: done.id, to_list_id: backlog.id } + params = { board_id: board1.id, from_list_id: done.id, to_list_id: backlog.id } described_class.new(project, user, params).execute(issue) issue.reload @@ -124,7 +128,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving to same list' do let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { from_list_id: list1.id, to_list_id: list1.id } } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } it 'returns false' do expect(described_class.new(project, user, params).execute(issue)).to eq false diff --git a/spec/services/boards/list_service_spec.rb b/spec/services/boards/list_service_spec.rb new file mode 100644 index 00000000000..dff33e4bcbb --- /dev/null +++ b/spec/services/boards/list_service_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Boards::ListService, services: true do + describe '#execute' do + let(:project) { create(:empty_project) } + + subject(:service) { described_class.new(project, double) } + + context 'when project does not have a board' do + it 'creates a new project board' do + expect { service.execute }.to change(project.boards, :count).by(1) + end + + it 'delegates the project board creation to Boards::CreateService' do + expect_any_instance_of(Boards::CreateService).to receive(:execute).once + + service.execute + end + end + + context 'when project has a board' do + before do + create(:board, project: project) + end + + it 'does not create a new board' do + expect { service.execute }.not_to change(project.boards, :count) + end + end + + it 'returns project boards' do + board = create(:board, project: project) + + expect(service.execute).to match_array [board] + end + end +end diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb index bff9c1fd1fe..e7806add916 100644 --- a/spec/services/boards/lists/create_service_spec.rb +++ b/spec/services/boards/lists/create_service_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Boards::Lists::CreateService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let(:label) { create(:label, project: project, name: 'in-progress') } @@ -11,7 +11,7 @@ describe Boards::Lists::CreateService, services: true do context 'when board lists is empty' do it 'creates a new list at beginning of the list' do - list = service.execute + list = service.execute(board) expect(list.position).to eq 0 end @@ -19,7 +19,7 @@ describe Boards::Lists::CreateService, services: true do context 'when board lists has backlog, and done lists' do it 'creates a new list at beginning of the list' do - list = service.execute + list = service.execute(board) expect(list.position).to eq 0 end @@ -30,7 +30,7 @@ describe Boards::Lists::CreateService, services: true do create(:list, board: board, position: 0) create(:list, board: board, position: 1) - list = service.execute + list = service.execute(board) expect(list.position).to eq 2 end @@ -40,7 +40,7 @@ describe Boards::Lists::CreateService, services: true do it 'creates a new list at end of the label lists' do list1 = create(:list, board: board, position: 0) - list2 = service.execute + list2 = service.execute(board) expect(list1.reload.position).to eq 0 expect(list2.reload.position).to eq 1 @@ -52,7 +52,7 @@ describe Boards::Lists::CreateService, services: true do label = create(:label, name: 'in-development') service = described_class.new(project, user, label_id: label.id) - expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + expect { service.execute(board) }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/spec/services/boards/lists/destroy_service_spec.rb b/spec/services/boards/lists/destroy_service_spec.rb index 474c4512471..628caf03476 100644 --- a/spec/services/boards/lists/destroy_service_spec.rb +++ b/spec/services/boards/lists/destroy_service_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Boards::Lists::DestroyService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } context 'when list type is label' do @@ -15,11 +15,11 @@ describe Boards::Lists::DestroyService, services: true do end it 'decrements position of higher lists' do - backlog = project.board.backlog_list + backlog = 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 = project.board.done_list + done = 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 = project.board.backlog_list + list = 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 = project.board.done_list + list = 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/generate_service_spec.rb b/spec/services/boards/lists/generate_service_spec.rb index 9fd39122737..8b2f5e81338 100644 --- a/spec/services/boards/lists/generate_service_spec.rb +++ b/spec/services/boards/lists/generate_service_spec.rb @@ -2,15 +2,15 @@ require 'spec_helper' describe Boards::Lists::GenerateService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } subject(:service) { described_class.new(project, user) } context 'when board lists is empty' do it 'creates the default lists' do - expect { service.execute }.to change(board.lists, :count).by(4) + expect { service.execute(board) }.to change(board.lists, :count).by(2) end end @@ -18,22 +18,21 @@ describe Boards::Lists::GenerateService, services: true do it 'does not creates the default lists' do create(:list, board: board) - expect { service.execute }.not_to change(board.lists, :count) + expect { service.execute(board) }.not_to change(board.lists, :count) end end context 'when project labels does not contains any list label' do it 'creates labels' do - expect { service.execute }.to change(project.labels, :count).by(4) + expect { service.execute(board) }.to change(project.labels, :count).by(2) end end context 'when project labels contains some of list label' do it 'creates the missing labels' do - create(:label, project: project, name: 'Development') - create(:label, project: project, name: 'Ready') + create(:label, project: project, name: 'Doing') - expect { service.execute }.to change(project.labels, :count).by(2) + expect { service.execute(board) }.to change(project.labels, :count).by(1) end end end diff --git a/spec/services/boards/lists/list_service_spec.rb b/spec/services/boards/lists/list_service_spec.rb new file mode 100644 index 00000000000..334cee3f06d --- /dev/null +++ b/spec/services/boards/lists/list_service_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe Boards::Lists::ListService, services: true do + describe '#execute' do + it "returns board's lists" do + project = create(:empty_project) + board = create(:board, project: project) + label = create(:label, project: project) + list = create(:list, board: board, label: label) + + service = described_class.new(project, double) + + expect(service.execute(board)).to eq [board.backlog_list, list, board.done_list] + end + end +end diff --git a/spec/services/boards/lists/move_service_spec.rb b/spec/services/boards/lists/move_service_spec.rb index 102ed67449d..63fa0bb8c5f 100644 --- a/spec/services/boards/lists/move_service_spec.rb +++ b/spec/services/boards/lists/move_service_spec.rb @@ -2,16 +2,16 @@ require 'spec_helper' describe Boards::Lists::MoveService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } - let!(:backlog) { project.board.backlog_list } + let!(:backlog) { create(:backlog_list, board: board) } 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) { project.board.done_list } + let!(:done) { create(:done_list, board: board) } 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/compare_service_spec.rb b/spec/services/compare_service_spec.rb new file mode 100644 index 00000000000..3760f19aaa2 --- /dev/null +++ b/spec/services/compare_service_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe CompareService, services: true do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:service) { described_class.new } + + describe '#execute' do + context 'compare with base, like feature...fix' do + subject { service.execute(project, 'feature', project, 'fix', straight: false) } + + it { expect(subject.diffs.size).to eq(1) } + end + + context 'straight compare, like feature..fix' do + subject { service.execute(project, 'feature', project, 'fix', straight: true) } + + it { expect(subject.diffs.size).to eq(3) } + end + end +end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 343b4385bf2..5fe56e7725f 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -84,11 +84,22 @@ describe CreateDeploymentService, services: true do expect(subject).to be_persisted end end + + context 'when project was removed' do + let(:project) { nil } + + it 'does not create deployment or environment' do + expect { subject }.not_to raise_error + + expect(Environment.count).to be_zero + expect(Deployment.count).to be_zero + end + end end describe 'processing of builds' do let(:environment) { nil } - + shared_examples 'does not create environment and deployment' do it 'does not create a new environment' do expect { subject }.not_to change { Environment.count } @@ -133,12 +144,12 @@ describe CreateDeploymentService, services: true do context 'without environment specified' do let(:build) { create(:ci_build, project: project) } - + it_behaves_like 'does not create environment and deployment' do subject { build.success } end end - + context 'when environment is specified' do let(:pipeline) { create(:ci_pipeline, project: project) } let(:build) { create(:ci_build, pipeline: pipeline, environment: 'production', options: options) } diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index d019e50649f..d3c37c7820f 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -41,7 +41,7 @@ describe Files::UpdateService do it "returns a hash with the :success status " do results = subject.execute - expect(results).to match({ status: :success }) + expect(results[:status]).to match(:success) end it "updates the file with the new contents" do @@ -69,7 +69,7 @@ describe Files::UpdateService do it "returns a hash with the :success status " do results = subject.execute - expect(results).to match({ status: :success }) + expect(results[:status]).to match(:success) end it "updates the file with the new contents" do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 22991c5bc86..dd2a9e9903a 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -184,8 +184,8 @@ describe GitPushService, services: true do context "Updates merge requests" do it "when pushing a new branch for the first time" do - expect(project).to receive(:update_merge_requests). - with(@blankrev, 'newrev', 'refs/heads/master', user) + expect(UpdateMergeRequestsWorker).to receive(:perform_async). + with(project.id, user.id, @blankrev, 'newrev', 'refs/heads/master') execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end end @@ -448,6 +448,8 @@ describe GitPushService, services: true do let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } before do + # project.create_jira_service doesn't seem to invalidate the cache here + project.has_external_issue_tracker = true jira_service_settings WebMock.stub_request(:post, jira_api_transition_url) diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb new file mode 100644 index 00000000000..7aeb95a15ea --- /dev/null +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe MergeRequests::AssignIssuesService, services: true do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, :simple, source_project: project, author: user, description: "fixes #{issue.to_reference}") } + let(:service) { described_class.new(project, user, merge_request: merge_request) } + + before do + project.team << [user, :developer] + end + + it 'finds unassigned issues fixed in merge request' do + expect(service.assignable_issues.map(&:id)).to include(issue.id) + end + + it 'ignores issues already assigned to any user' do + issue.update!(assignee: create(:user)) + + expect(service.assignable_issues).to be_empty + end + + it 'ignores issues the user cannot update assignee on' do + project.team.truncate + + expect(service.assignable_issues).to be_empty + end + + it 'ignores all issues unless current_user is merge_request.author' do + merge_request.update!(author: create(:user)) + + expect(service.assignable_issues).to be_empty + end + + it 'accepts precomputed data for closes_issues' do + issue2 = create(:issue, project: project) + service2 = described_class.new(project, + user, + merge_request: merge_request, + closes_issues: [issue, issue2]) + + expect(service2.assignable_issues.count).to eq 2 + end + + it 'assigns these to the merge request owner' do + expect { service.execute }.to change { issue.reload.assignee }.to(user) + end +end diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 0d586e2216b..3a3f07ddcb9 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -52,12 +52,28 @@ describe MergeRequests::BuildService, services: true do end end - context 'no commits in the diff' do - let(:commits) { [] } + context 'same source and target branch' do + let(:source_branch) { 'master' } it 'forbids the merge request from being created' do expect(merge_request.can_be_created).to eq(false) end + + it 'adds an error message to the merge request' do + expect(merge_request.errors).to contain_exactly('You must select different branches') + end + end + + context 'no commits in the diff' do + let(:commits) { [] } + + it 'allows the merge request to be created' do + expect(merge_request.can_be_created).to eq(true) + end + + it 'adds a WIP prefix to the merge request title' do + expect(merge_request.title).to eq('WIP: Feature branch') + end end context 'one commit in the diff' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index e49a0d5e553..ee53e110aee 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -60,7 +60,10 @@ describe MergeRequests::MergeService, services: true do let(:jira_tracker) { project.create_jira_service } - before { jira_service_settings } + before do + project.update_attributes!(has_external_issue_tracker: true) + jira_service_settings + end it 'closes issues on JIRA issue tracker' do jira_issue = ExternalIssue.new('JIRA-123', project) diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb index 520e906b21f..b80cfd8f450 100644 --- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -58,61 +58,83 @@ describe MergeRequests::MergeWhenBuildSucceedsService do end describe "#trigger" do - context 'build with ref' do - let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch, status: "success") } + let(:merge_request_ref) { mr_merge_if_green_enabled.source_branch } + let(:merge_request_head) do + project.commit(mr_merge_if_green_enabled.source_branch).id + end - it "merges all merge requests with merge when build succeeds enabled" do - allow_any_instance_of(MergeRequest).to receive(:pipeline).and_return(pipeline) - allow(pipeline).to receive(:success?).and_return(true) + context 'when triggered by pipeline with valid ref and sha' do + let(:triggering_pipeline) do + create(:ci_pipeline, project: project, ref: merge_request_ref, + sha: merge_request_head, status: 'success') + end + it "merges all merge requests with merge when build succeeds enabled" do expect(MergeWorker).to receive(:perform_async) - service.trigger(build) + service.trigger(triggering_pipeline) end end - context 'triggered by an old build' do - let(:old_build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch, status: "success") } - let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch, status: "success") } - - it "merges all merge requests with merge when build succeeds enabled" do - allow_any_instance_of(MergeRequest).to receive(:pipeline).and_return(pipeline) - allow(pipeline).to receive(:success?).and_return(true) - allow(old_build).to receive(:sha).and_return('1234abcdef') + context 'when triggered by an old pipeline' do + let(:old_pipeline) do + create(:ci_pipeline, project: project, ref: merge_request_ref, + sha: '1234abcdef', status: 'success') + end + it 'it does not merge merge request' do expect(MergeWorker).not_to receive(:perform_async) - service.trigger(old_build) + service.trigger(old_pipeline) end end - context 'commit status without ref' do - let(:commit_status) { create(:generic_commit_status, status: 'success') } - - before { mr_merge_if_green_enabled } - - it "doesn't merge a requests for status on other branch" do - allow(project.repository).to receive(:branch_names_contains).with(commit_status.sha).and_return([]) + context 'when triggered by pipeline from a different branch' do + let(:unrelated_pipeline) do + create(:ci_pipeline, project: project, ref: 'feature', + sha: merge_request_head, status: 'success') + end + it 'does not merge request' do expect(MergeWorker).not_to receive(:perform_async) - service.trigger(commit_status) + service.trigger(unrelated_pipeline) end + end + end - it 'discovers branches and merges all merge requests when status is success' do - allow(project.repository).to receive(:branch_names_contains). - with(commit_status.sha).and_return([mr_merge_if_green_enabled.source_branch]) - allow(pipeline).to receive(:success?).and_return(true) - allow_any_instance_of(MergeRequest).to receive(:pipeline).and_return(pipeline) - allow(pipeline).to receive(:success?).and_return(true) + describe "#cancel" do + before do + service.cancel(mr_merge_if_green_enabled) + end - expect(MergeWorker).to receive(:perform_async) - service.trigger(commit_status) - end + it "resets all the merge_when_build_succeeds params" do + expect(mr_merge_if_green_enabled.merge_when_build_succeeds).to be_falsey + expect(mr_merge_if_green_enabled.merge_params).to eq({}) + expect(mr_merge_if_green_enabled.merge_user).to be nil end - context 'properly handles multiple stages' do + it 'Posts a system note' do + note = mr_merge_if_green_enabled.notes.last + expect(note.note).to include 'Canceled the automatic merge' + end + end + + describe 'pipeline integration' do + context 'when there are multiple stages in the pipeline' do let(:ref) { mr_merge_if_green_enabled.source_branch } - let!(:build) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'build', stage: 'build') } - let!(:test) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'test', stage: 'test') } - let(:pipeline) { create(:ci_empty_pipeline, ref: mr_merge_if_green_enabled.source_branch, project: project) } + let(:sha) { project.commit(ref).id } + + let(:pipeline) do + create(:ci_empty_pipeline, ref: ref, sha: sha, project: project) + end + + let!(:build) do + create(:ci_build, :created, pipeline: pipeline, ref: ref, + name: 'build', stage: 'build') + end + + let!(:test) do + create(:ci_build, :created, pipeline: pipeline, ref: ref, + name: 'test', stage: 'test') + end before do # This behavior of MergeRequest: we instantiate a new object @@ -121,34 +143,19 @@ describe MergeRequests::MergeWhenBuildSucceedsService do end end - it "doesn't merge if some stages failed" do + it "doesn't merge if any of stages failed" do expect(MergeWorker).not_to receive(:perform_async) + build.success test.drop end - it 'merge when all stages succeeded' do + it 'merges when all stages succeeded' do expect(MergeWorker).to receive(:perform_async) + build.success test.success end end end - - describe "#cancel" do - before do - service.cancel(mr_merge_if_green_enabled) - end - - it "resets all the merge_when_build_succeeds params" do - expect(mr_merge_if_green_enabled.merge_when_build_succeeds).to be_falsey - expect(mr_merge_if_green_enabled.merge_params).to eq({}) - expect(mr_merge_if_green_enabled.merge_user).to be nil - end - - it 'Posts a system note' do - note = mr_merge_if_green_enabled.notes.last - expect(note.note).to include 'Canceled the automatic merge' - end - end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 59d3912018a..e515bc9f89c 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -62,7 +62,8 @@ describe MergeRequests::RefreshService, services: true do it { expect(@merge_request.notes).not_to be_empty } it { expect(@merge_request).to be_open } - it { expect(@merge_request.merge_when_build_succeeds).to be_falsey} + it { expect(@merge_request.merge_when_build_succeeds).to be_falsey } + it { expect(@merge_request.diff_head_sha).to eq(@newrev) } it { expect(@fork_merge_request).to be_open } it { expect(@fork_merge_request.notes).to be_empty } it { expect(@build_failed_todo).to be_done } @@ -118,7 +119,7 @@ describe MergeRequests::RefreshService, services: true do it { expect(@merge_request.notes).to be_empty } it { expect(@merge_request).to be_open } - it { expect(@fork_merge_request.notes.last.note).to include('Added 4 commits') } + it { expect(@fork_merge_request.notes.last.note).to include('Added 28 commits') } it { expect(@fork_merge_request).to be_open } it { expect(@build_failed_todo).to be_pending } it { expect(@fork_build_failed_todo).to be_pending } @@ -169,7 +170,7 @@ describe MergeRequests::RefreshService, services: true do notes = @fork_merge_request.notes.reorder(:created_at).map(&:note) expect(notes[0]).to include('Restored source branch `master`') - expect(notes[1]).to include('Added 4 commits') + expect(notes[1]).to include('Added 28 commits') expect(@fork_merge_request).to be_open end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 33db34c0f62..2433a7dad06 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -17,6 +17,7 @@ describe MergeRequests::UpdateService, services: true do before do project.team << [user, :master] project.team << [user2, :developer] + project.team << [user3, :developer] end describe 'execute' do @@ -104,6 +105,18 @@ describe MergeRequests::UpdateService, services: true do expect(note).not_to be_nil expect(note.note).to eq 'Target branch changed from `master` to `target`' end + + context 'when not including source branch removal options' do + before do + opts.delete(:force_remove_source_branch) + end + + it 'maintains the original options' do + update_merge_request(opts) + + expect(@merge_request.merge_params["force_remove_source_branch"]).to eq("1") + end + end end context 'todos' do @@ -188,6 +201,11 @@ describe MergeRequests::UpdateService, services: true do let!(:non_subscriber) { create(:user) } let!(:subscriber) { create(:user).tap { |u| label.toggle_subscription(u) } } + before do + project.team << [non_subscriber, :developer] + project.team << [subscriber, :developer] + end + it 'sends notifications for subscribers of newly added labels' do opts = { label_ids: [label.id] } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index d820646ebdf..699b9925b4e 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -331,7 +331,7 @@ describe NotificationService, services: true do describe '#new_note' do it "records sent notifications" do # Ensure create SentNotification by noteable = merge_request 6 times, not noteable = note - expect(SentNotification).to receive(:record_note).with(note, any_args).exactly(4).times.and_call_original + expect(SentNotification).to receive(:record_note).with(note, any_args).exactly(3).times.and_call_original notification.new_note(note) @@ -1169,6 +1169,61 @@ describe NotificationService, services: true do end end + context 'guest user in private project' do + let(:private_project) { create(:empty_project, :private) } + let(:guest) { create(:user) } + let(:developer) { create(:user) } + let(:assignee) { create(:user) } + let(:merge_request) { create(:merge_request, source_project: private_project, assignee: assignee) } + let(:merge_request1) { create(:merge_request, source_project: private_project, assignee: assignee, description: "cc @#{guest.username}") } + let(:note) { create(:note, noteable: merge_request, project: private_project) } + + before do + private_project.team << [assignee, :developer] + private_project.team << [developer, :developer] + private_project.team << [guest, :guest] + + ActionMailer::Base.deliveries.clear + end + + it 'filters out guests when new note is created' do + expect(SentNotification).to receive(:record).with(merge_request, any_args).exactly(1).times + + notification.new_note(note) + + should_not_email(guest) + should_email(assignee) + end + + it 'filters out guests when new merge request is created' do + notification.new_merge_request(merge_request1, @u_disabled) + + should_not_email(guest) + should_email(assignee) + end + + it 'filters out guests when merge request is closed' do + notification.close_mr(merge_request, developer) + + should_not_email(guest) + should_email(assignee) + end + + it 'filters out guests when merge request is reopened' do + notification.reopen_mr(merge_request, developer) + + should_not_email(guest) + should_email(assignee) + end + + it 'filters out guests when merge request is merged' do + notification.merge_mr(merge_request, developer) + + should_not_email(guest) + should_email(assignee) + end + end + def build_team(project) @u_watcher = create_global_setting_for(create(:user), :watch) @u_participating = create_global_setting_for(create(:user), :participating) diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index ae4d286d250..b57e338b782 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -86,6 +86,25 @@ describe SlashCommands::InterpretService, services: true do end end + shared_examples 'multiple label command' do + it 'fetches label ids and populates add_label_ids if content contains multiple /label' do + bug # populate the label + inprogress # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(add_label_ids: [inprogress.id, bug.id]) + end + end + + shared_examples 'multiple label with same argument' do + it 'prevents duplicate label ids and populates add_label_ids if content contains multiple /label' do + inprogress # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(add_label_ids: [inprogress.id]) + end + end + shared_examples 'unlabel command' do it 'fetches label ids and populates remove_label_ids if content contains /unlabel' do issuable.update(label_ids: [inprogress.id]) # populate the label @@ -95,6 +114,15 @@ describe SlashCommands::InterpretService, services: true do end end + shared_examples 'multiple unlabel command' do + it 'fetches label ids and populates remove_label_ids if content contains mutiple /unlabel' do + issuable.update(label_ids: [inprogress.id, bug.id]) # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(remove_label_ids: [inprogress.id, bug.id]) + end + end + shared_examples 'unlabel command with no argument' do it 'populates label_ids: [] if content contains /unlabel with no arguments' do issuable.update(label_ids: [inprogress.id]) # populate the label @@ -285,6 +313,16 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end + it_behaves_like 'multiple label command' do + let(:content) { %(/label ~"#{inprogress.title}" \n/label ~#{bug.title}) } + let(:issuable) { issue } + end + + it_behaves_like 'multiple label with same argument' do + let(:content) { %(/label ~"#{inprogress.title}" \n/label ~#{inprogress.title}) } + let(:issuable) { issue } + end + it_behaves_like 'unlabel command' do let(:content) { %(/unlabel ~"#{inprogress.title}") } let(:issuable) { issue } @@ -295,6 +333,11 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end + it_behaves_like 'multiple unlabel command' do + let(:content) { %(/unlabel ~"#{inprogress.title}" \n/unlabel ~#{bug.title}) } + let(:issuable) { issue } + end + it_behaves_like 'unlabel command with no argument' do let(:content) { %(/unlabel) } let(:issuable) { issue } diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index c22dd9ab77a..b4ba28dfe8e 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -54,7 +54,7 @@ describe SystemNoteService, services: true do it 'adds a message line for each commit' do new_commits.each_with_index do |commit, i| # Skip the header - expect(note_lines[i + 1]).to eq "* #{commit.short_id} - #{commit.title}" + expect(HTMLEntities.new.decode(note_lines[i + 1])).to eq "* #{commit.short_id} - #{commit.title}" end end end @@ -81,7 +81,7 @@ describe SystemNoteService, services: true do end it 'includes a commit count' do - expect(summary_line).to end_with " - 2 commits from branch `feature`" + expect(summary_line).to end_with " - 26 commits from branch `feature`" end end @@ -91,7 +91,7 @@ describe SystemNoteService, services: true do end it 'includes a commit count' do - expect(summary_line).to end_with " - 2 commits from branch `feature`" + expect(summary_line).to end_with " - 26 commits from branch `feature`" end end @@ -531,13 +531,13 @@ describe SystemNoteService, services: true do include JiraServiceHelper describe 'JIRA integration' do - let(:project) { create(:project) } + let(:project) { create(:jira_project) } let(:author) { create(:user) } let(:issue) { create(:issue, project: project) } let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) } let(:jira_issue) { ExternalIssue.new("JIRA-1", project)} - let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } - let(:commit) { project.commit } + let(:jira_tracker) { project.jira_service } + let(:commit) { project.repository.commits('master').find { |commit| commit.id == '5937ac0a7beb003549fc5fd26fc247adbce4a52e' } } context 'in JIRA issue tracker' do before do @@ -545,10 +545,6 @@ describe SystemNoteService, services: true do WebMock.stub_request(:post, jira_api_comment_url) end - after do - jira_tracker.destroy! - end - describe "new reference" do before do WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments) @@ -561,7 +557,7 @@ describe SystemNoteService, services: true do describe "existing reference" do before do - message = %Q{[#{author.name}|http://localhost/u/#{author.username}] mentioned this issue in [a commit of #{project.path_with_namespace}|http://localhost/#{project.path_with_namespace}/commit/#{commit.id}]:\\n'#{commit.title}'} + message = %Q{[#{author.name}|http://localhost/#{author.username}] mentioned this issue in [a commit of #{project.path_with_namespace}|http://localhost/#{project.path_with_namespace}/commit/#{commit.id}]:\\n'#{commit.title}'} WebMock.stub_request(:get, jira_api_comment_url).to_return(body: %Q({"comments":[{"body":"#{message}"}]})) end @@ -578,10 +574,6 @@ describe SystemNoteService, services: true do WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments) end - after do - jira_tracker.destroy! - end - subject { described_class.cross_reference(jira_issue, issue, author) } it { is_expected.to eq(jira_status_message) } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index b41f6f14fbd..ed55791d24e 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -345,7 +345,7 @@ describe TodoService, services: true do service.new_merge_request(mr_assigned, author) should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) - should_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) + should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) @@ -357,7 +357,7 @@ describe TodoService, services: true do service.update_merge_request(mr_assigned, author) should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) - should_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) + should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) @@ -381,6 +381,7 @@ describe TodoService, services: true do should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) + should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) end it 'does not raise an error when description not change' do @@ -430,6 +431,11 @@ describe TodoService, services: true do should_create_todo(user: john_doe, target: mr_assigned, author: john_doe, action: Todo::ASSIGNED) end + + it 'does not create a todo for guests' do + service.reassigned_merge_request(mr_assigned, author) + should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) + end end describe '#merge_merge_request' do @@ -441,6 +447,11 @@ describe TodoService, services: true do expect(first_todo.reload).to be_done expect(second_todo.reload).to be_done end + + it 'does not create todo for guests' do + service.merge_merge_request(mr_assigned, john_doe) + should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) + end end describe '#new_award_emoji' do @@ -495,6 +506,13 @@ describe TodoService, services: true do should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::MENTIONED, note: legacy_diff_note_on_merge_request) end + + it 'does not create todo for guests' do + note_on_merge_request = create :note_on_merge_request, project: project, noteable: mr_assigned, note: mentions + service.new_note(note_on_merge_request, author) + + should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) + end end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 0097dbf8fad..ad8ae763f6d 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -5,6 +5,8 @@ module TestEnv # When developing the seed repository, comment out the branch you will modify. BRANCH_SHA = { + 'not-merged-branch' => 'b83d6e3', + 'branch-merged' => '498214d', 'empty-branch' => '7efb185', 'ends-with.json' => '98b0d8b', 'flatten-dir' => 'e56497b', @@ -14,7 +16,8 @@ module TestEnv 'improve/awesome' => '5937ac0', 'markdown' => '0ed8c6c', 'lfs' => 'be93687', - 'master' => '5937ac0', + 'master' => 'b83d6e3', + 'merge-test' => '5937ac0', "'test'" => 'e56497b', 'orphaned-branch' => '45127a9', 'binary-encoding' => '7b1cf43', diff --git a/spec/tasks/gitlab/users_rake_spec.rb b/spec/tasks/gitlab/users_rake_spec.rb new file mode 100644 index 00000000000..e6ebef82b78 --- /dev/null +++ b/spec/tasks/gitlab/users_rake_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' +require 'rake' + +describe 'gitlab:users namespace rake task' do + let(:enable_registry) { true } + + before :all do + Rake.application.rake_require 'tasks/gitlab/task_helpers' + Rake.application.rake_require 'tasks/gitlab/users' + + # empty task as env is already loaded + Rake::Task.define_task :environment + end + + def run_rake_task(task_name) + Rake::Task[task_name].reenable + Rake.application.invoke_task task_name + end + + describe 'clear_all_authentication_tokens' do + before do + # avoid writing task output to spec progress + allow($stdout).to receive :write + end + + context 'gitlab version' do + it 'clears the authentication token for all users' do + create_list(:user, 2) + + expect(User.pluck(:authentication_token)).to all(be_present) + + run_rake_task('gitlab:users:clear_all_authentication_tokens') + + expect(User.pluck(:authentication_token)).to all(be_nil) + end + end + end +end diff --git a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb deleted file mode 100644 index 86980f59cd8..00000000000 --- a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'spec_helper' - -describe 'projects/merge_requests/widget/_heading' do - include Devise::Test::ControllerHelpers - - context 'when released to an environment' do - let(:project) { merge_request.target_project } - let(:merge_request) { create(:merge_request, :merged) } - let(:environment) { create(:environment, project: project) } - let!(:deployment) do - create(:deployment, environment: environment, sha: project.commit('master').id) - end - - before do - assign(:merge_request, merge_request) - assign(:project, project) - - allow(view).to receive(:can?).and_return(true) - - render - end - - it 'displays that the environment is deployed' do - expect(rendered).to match("Deployed to") - expect(rendered).to match("#{environment.name}") - end - end -end diff --git a/spec/workers/build_coverage_worker_spec.rb b/spec/workers/build_coverage_worker_spec.rb new file mode 100644 index 00000000000..ba20488f663 --- /dev/null +++ b/spec/workers/build_coverage_worker_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe BuildCoverageWorker do + describe '#perform' do + context 'when build exists' do + let!(:build) { create(:ci_build) } + + it 'updates code coverage' do + expect_any_instance_of(Ci::Build) + .to receive(:update_coverage) + + described_class.new.perform(build.id) + end + end + + context 'when build does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end diff --git a/spec/workers/build_finished_worker_spec.rb b/spec/workers/build_finished_worker_spec.rb new file mode 100644 index 00000000000..2868167c7d4 --- /dev/null +++ b/spec/workers/build_finished_worker_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe BuildFinishedWorker do + describe '#perform' do + context 'when build exists' do + let(:build) { create(:ci_build) } + + it 'calculates coverage and calls hooks' do + expect(BuildCoverageWorker) + .to receive(:new).ordered.and_call_original + expect(BuildHooksWorker) + .to receive(:new).ordered.and_call_original + + expect_any_instance_of(BuildCoverageWorker) + .to receive(:perform) + expect_any_instance_of(BuildHooksWorker) + .to receive(:perform) + + described_class.new.perform(build.id) + end + end + + context 'when build does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end diff --git a/spec/workers/build_hooks_worker_spec.rb b/spec/workers/build_hooks_worker_spec.rb new file mode 100644 index 00000000000..97654a93f5c --- /dev/null +++ b/spec/workers/build_hooks_worker_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe BuildHooksWorker do + describe '#perform' do + context 'when build exists' do + let!(:build) { create(:ci_build) } + + it 'calls build hooks' do + expect_any_instance_of(Ci::Build) + .to receive(:execute_hooks) + + described_class.new.perform(build.id) + end + end + + context 'when build does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end diff --git a/spec/workers/build_success_worker_spec.rb b/spec/workers/build_success_worker_spec.rb new file mode 100644 index 00000000000..dba70883130 --- /dev/null +++ b/spec/workers/build_success_worker_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe BuildSuccessWorker do + describe '#perform' do + context 'when build exists' do + context 'when build belogs to the environment' do + let!(:build) { create(:ci_build, environment: 'production') } + + it 'executes deployment service' do + expect_any_instance_of(CreateDeploymentService) + .to receive(:execute) + + described_class.new.perform(build.id) + end + end + + context 'when build is not associated with project' do + let!(:build) { create(:ci_build, project: nil) } + + it 'does not create deployment' do + expect_any_instance_of(CreateDeploymentService) + .not_to receive(:execute) + + described_class.new.perform(build.id) + end + end + end + + context 'when build does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb index 7ca2c29da1c..036d037f3f9 100644 --- a/spec/workers/emails_on_push_worker_spec.rb +++ b/spec/workers/emails_on_push_worker_spec.rb @@ -57,7 +57,7 @@ describe EmailsOnPushWorker do end it "sends a mail with the correct subject" do - expect(email.subject).to include('Change some files') + expect(email.subject).to include('adds bar folder and branch-test text file') end it "mentions force pushing in the body" do @@ -73,7 +73,7 @@ describe EmailsOnPushWorker do before { perform } it "sends a mail with the correct subject" do - expect(email.subject).to include('Change some files') + expect(email.subject).to include('adds bar folder and branch-test text file') end it "does not mention force pushing in the body" do diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb index 7d6668920c0..73cbadc13d9 100644 --- a/spec/workers/expire_build_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -5,65 +5,42 @@ describe ExpireBuildArtifactsWorker do let(:worker) { described_class.new } + before { Sidekiq::Worker.clear_all } + describe '#perform' do before { build } - subject! { worker.perform } + subject! do + Sidekiq::Testing.fake! { worker.perform } + end context 'with expired artifacts' do let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } - it 'does expire' do - expect(build.reload.artifacts_expired?).to be_truthy - end - - it 'does remove files' do - expect(build.reload.artifacts_file.exists?).to be_falsey - end - - it 'does nullify artifacts_file column' do - expect(build.reload.artifacts_file_identifier).to be_nil + it 'enqueues that build' do + expect(jobs_enqueued.size).to eq(1) + expect(jobs_enqueued[0]["args"]).to eq([build.id]) end end context 'with not yet expired artifacts' do let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days) } - it 'does not expire' do - expect(build.reload.artifacts_expired?).to be_falsey - end - - it 'does not remove files' do - expect(build.reload.artifacts_file.exists?).to be_truthy - end - - it 'does not nullify artifacts_file column' do - expect(build.reload.artifacts_file_identifier).not_to be_nil + it 'does not enqueue that build' do + expect(jobs_enqueued.size).to eq(0) end end context 'without expire date' do let(:build) { create(:ci_build, :artifacts) } - it 'does not expire' do - expect(build.reload.artifacts_expired?).to be_falsey - end - - it 'does not remove files' do - expect(build.reload.artifacts_file.exists?).to be_truthy - end - - it 'does not nullify artifacts_file column' do - expect(build.reload.artifacts_file_identifier).not_to be_nil + it 'does not enqueue that build' do + expect(jobs_enqueued.size).to eq(0) end end - context 'for expired artifacts' do - let(:build) { create(:ci_build, artifacts_expire_at: Time.now - 7.days) } - - it 'is still expired' do - expect(build.reload.artifacts_expired?).to be_truthy - end + def jobs_enqueued + Sidekiq::Queues.jobs_by_worker['ExpireBuildInstanceArtifactsWorker'] end end end diff --git a/spec/workers/expire_build_instance_artifacts_worker_spec.rb b/spec/workers/expire_build_instance_artifacts_worker_spec.rb new file mode 100644 index 00000000000..2b140f2ba28 --- /dev/null +++ b/spec/workers/expire_build_instance_artifacts_worker_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe ExpireBuildInstanceArtifactsWorker do + include RepoHelpers + + let(:worker) { described_class.new } + + describe '#perform' do + before { build } + + subject! { worker.perform(build.id) } + + context 'with expired artifacts' do + let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } + + it 'does expire' do + expect(build.reload.artifacts_expired?).to be_truthy + end + + it 'does remove files' do + expect(build.reload.artifacts_file.exists?).to be_falsey + end + + it 'does nullify artifacts_file column' do + expect(build.reload.artifacts_file_identifier).to be_nil + end + end + + context 'with not yet expired artifacts' do + let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days) } + + it 'does not expire' do + expect(build.reload.artifacts_expired?).to be_falsey + end + + it 'does not remove files' do + expect(build.reload.artifacts_file.exists?).to be_truthy + end + + it 'does not nullify artifacts_file column' do + expect(build.reload.artifacts_file_identifier).not_to be_nil + end + end + + context 'without expire date' do + let(:build) { create(:ci_build, :artifacts) } + + it 'does not expire' do + expect(build.reload.artifacts_expired?).to be_falsey + end + + it 'does not remove files' do + expect(build.reload.artifacts_file.exists?).to be_truthy + end + + it 'does not nullify artifacts_file column' do + expect(build.reload.artifacts_file_identifier).not_to be_nil + end + end + + context 'for expired artifacts' do + let(:build) { create(:ci_build, artifacts_expire_at: Time.now - 7.days) } + + it 'is still expired' do + expect(build.reload.artifacts_expired?).to be_truthy + end + end + end +end diff --git a/spec/workers/pipeline_hooks_worker_spec.rb b/spec/workers/pipeline_hooks_worker_spec.rb new file mode 100644 index 00000000000..035e329839f --- /dev/null +++ b/spec/workers/pipeline_hooks_worker_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe PipelineHooksWorker do + describe '#perform' do + context 'when pipeline exists' do + let(:pipeline) { create(:ci_pipeline) } + + it 'executes hooks for the pipeline' do + expect_any_instance_of(Ci::Pipeline) + .to receive(:execute_hooks) + + described_class.new.perform(pipeline.id) + end + end + + context 'when pipeline does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end diff --git a/spec/workers/process_pipeline_worker_spec.rb b/spec/workers/pipeline_proccess_worker_spec.rb index 7b5f98d5763..86e9d7f6684 100644 --- a/spec/workers/process_pipeline_worker_spec.rb +++ b/spec/workers/pipeline_proccess_worker_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe ProcessPipelineWorker do +describe PipelineProcessWorker do describe '#perform' do context 'when pipeline exists' do let(:pipeline) { create(:ci_pipeline) } diff --git a/spec/workers/pipeline_success_worker_spec.rb b/spec/workers/pipeline_success_worker_spec.rb new file mode 100644 index 00000000000..5e31cc2c8e7 --- /dev/null +++ b/spec/workers/pipeline_success_worker_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe PipelineSuccessWorker do + describe '#perform' do + context 'when pipeline exists' do + let(:pipeline) { create(:ci_pipeline, status: 'success') } + + it 'performs "merge when pipeline succeeds"' do + expect_any_instance_of( + MergeRequests::MergeWhenBuildSucceedsService + ).to receive(:trigger) + + described_class.new.perform(pipeline.id) + end + end + + context 'when pipeline does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end diff --git a/spec/workers/update_pipeline_worker_spec.rb b/spec/workers/pipeline_update_worker_spec.rb index fadc42b22f0..0b456cfd0da 100644 --- a/spec/workers/update_pipeline_worker_spec.rb +++ b/spec/workers/pipeline_update_worker_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe UpdatePipelineWorker do +describe PipelineUpdateWorker do describe '#perform' do context 'when pipeline exists' do let(:pipeline) { create(:ci_pipeline) } diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index ffeaafe654a..984acdade36 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -92,7 +92,13 @@ describe PostReceive do allow(Project).to receive(:find_with_namespace).and_return(project) expect(project).to receive(:execute_hooks).twice expect(project).to receive(:execute_services).twice - expect(project).to receive(:update_merge_requests) + + PostReceive.new.perform(pwd(project), key_id, base64_changes) + end + + it "enqueues a UpdateMergeRequestsWorker job" do + allow(Project).to receive(:find_with_namespace).and_return(project) + expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args) PostReceive.new.perform(pwd(project), key_id, base64_changes) end diff --git a/spec/workers/trending_projects_worker_spec.rb b/spec/workers/trending_projects_worker_spec.rb new file mode 100644 index 00000000000..c3c6fdcf2d5 --- /dev/null +++ b/spec/workers/trending_projects_worker_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe TrendingProjectsWorker do + describe '#perform' do + it 'refreshes the trending projects' do + expect(TrendingProject).to receive(:refresh!) + + described_class.new.perform + end + end +end diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb new file mode 100644 index 00000000000..c78a69eda67 --- /dev/null +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe UpdateMergeRequestsWorker do + include RepoHelpers + + let(:project) { create(:project) } + let(:user) { create(:user) } + + subject { described_class.new } + + describe '#perform' do + let(:oldrev) { "123456" } + let(:newrev) { "789012" } + let(:ref) { "refs/heads/test" } + + def perform + subject.perform(project.id, user.id, oldrev, newrev, ref) + end + + it 'executes MergeRequests::RefreshService with expected values' do + expect(MergeRequests::RefreshService).to receive(:new).with(project, user).and_call_original + expect_any_instance_of(MergeRequests::RefreshService).to receive(:execute).with(oldrev, newrev, ref) + + perform + end + + it 'executes SystemHooksService with expected values' do + push_data = double('push_data') + expect(Gitlab::DataBuilder::Push).to receive(:build).with(project, user, oldrev, newrev, ref, []).and_return(push_data) + + system_hook_service = double('system_hook_service') + expect(SystemHooksService).to receive(:new).and_return(system_hook_service) + expect(system_hook_service).to receive(:execute_hooks).with(push_data, :push_hooks) + + perform + end + end +end |