diff options
Diffstat (limited to 'spec')
32 files changed, 1717 insertions, 364 deletions
diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb deleted file mode 100644 index a3a3309e15e..00000000000 --- a/spec/controllers/commit_controller_spec.rb +++ /dev/null @@ -1,246 +0,0 @@ -require 'spec_helper' - -describe Projects::CommitController do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:commit) { project.commit("master") } - let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' } - let(:master_pickable_commit) { project.commit(master_pickable_sha) } - - before do - sign_in(user) - project.team << [user, :master] - end - - describe "#show" do - shared_examples "export as" do |format| - it "should generally work" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id, - format: format) - - expect(response).to be_success - end - - it "should generate it" do - expect_any_instance_of(Commit).to receive(:"to_#{format}") - - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id, format: format) - end - - it "should render it" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id, format: format) - - expect(response.body).to eq(commit.send(:"to_#{format}")) - end - - it "should not escape Html" do - allow_any_instance_of(Commit).to receive(:"to_#{format}"). - and_return('HTML entities &<>" ') - - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id, format: format) - - expect(response.body).not_to include('&') - expect(response.body).not_to include('>') - expect(response.body).not_to include('<') - expect(response.body).not_to include('"') - end - end - - describe "as diff" do - include_examples "export as", :diff - let(:format) { :diff } - - it "should really only be a git diff" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id, - format: format) - - expect(response.body).to start_with("diff --git") - end - - it "should really only be a git diff without whitespace changes" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: '66eceea0db202bb39c4e445e8ca28689645366c5', - # id: commit.id, - format: format, - w: 1) - - expect(response.body).to start_with("diff --git") - # without whitespace option, there are more than 2 diff_splits - diff_splits = assigns(:diffs).first.diff.split("\n") - expect(diff_splits.length).to be <= 2 - end - end - - describe "as patch" do - include_examples "export as", :patch - let(:format) { :patch } - - it "should really be a git email patch" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id, - format: format) - - expect(response.body).to start_with("From #{commit.id}") - end - - it "should contain a git diff" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id, - format: format) - - expect(response.body).to match(/^diff --git/) - end - end - - context 'commit that removes a submodule' do - render_views - - let(:fork_project) { create(:forked_project_with_submodules) } - let(:commit) { fork_project.commit('remove-submodule') } - - before do - fork_project.team << [user, :master] - end - - it 'renders it' do - get(:show, - namespace_id: fork_project.namespace.to_param, - project_id: fork_project.to_param, - id: commit.id) - - expect(response).to be_success - end - end - end - - describe "#branches" do - it "contains branch and tags information" do - get(:branches, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id) - - expect(assigns(:branches)).to include("master", "feature_conflict") - expect(assigns(:tags)).to include("v1.1.0") - end - end - - describe '#revert' do - context 'when target branch is not provided' do - it 'should render the 404 page' do - post(:revert, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: commit.id) - - expect(response).not_to be_success - expect(response).to have_http_status(404) - end - end - - context 'when the revert was successful' do - it 'should redirect to the commits page' do - post(:revert, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - target_branch: 'master', - id: commit.id) - - expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master') - expect(flash[:notice]).to eq('The commit has been successfully reverted.') - end - end - - context 'when the revert failed' do - before do - post(:revert, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - target_branch: 'master', - id: commit.id) - end - - it 'should redirect to the commit page' do - # Reverting a commit that has been already reverted. - post(:revert, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - target_branch: 'master', - id: commit.id) - - expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, commit.id) - expect(flash[:alert]).to match('Sorry, we cannot revert this commit automatically.') - end - end - end - - describe '#cherry_pick' do - context 'when target branch is not provided' do - it 'should render the 404 page' do - post(:cherry_pick, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: master_pickable_commit.id) - - expect(response).not_to be_success - expect(response).to have_http_status(404) - end - end - - context 'when the cherry-pick was successful' do - it 'should redirect to the commits page' do - post(:cherry_pick, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - target_branch: 'master', - id: master_pickable_commit.id) - - expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master') - expect(flash[:notice]).to eq('The commit has been successfully cherry-picked.') - end - end - - context 'when the cherry_pick failed' do - before do - post(:cherry_pick, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - target_branch: 'master', - id: master_pickable_commit.id) - end - - it 'should redirect to the commit page' do - # Cherry-picking a commit that has been already cherry-picked. - post(:cherry_pick, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - target_branch: 'master', - id: master_pickable_commit.id) - - expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) - expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.') - end - end - end -end diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 6e3db10e451..3001d32e719 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -1,9 +1,29 @@ -require 'rails_helper' +require 'spec_helper' describe Projects::CommitController do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:commit) { project.commit("master") } + let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' } + let(:master_pickable_commit) { project.commit(master_pickable_sha) } + + before do + sign_in(user) + project.team << [user, :master] + end + describe 'GET show' do render_views + def go(extra_params = {}) + params = { + namespace_id: project.namespace.to_param, + project_id: project.to_param + } + + get :show, params.merge(extra_params) + end + let(:project) { create(:project) } before do @@ -15,7 +35,7 @@ describe Projects::CommitController do context 'with valid id' do it 'responds with 200' do - go id: project.commit.id + go(id: commit.id) expect(response).to be_ok end @@ -23,27 +43,274 @@ describe Projects::CommitController do context 'with invalid id' do it 'responds with 404' do - go id: project.commit.id.reverse + go(id: commit.id.reverse) expect(response).to be_not_found end end it 'handles binary files' do - get(:show, + go(id: TestEnv::BRANCH_SHA['binary-encoding'], format: 'html') + + expect(response).to be_success + end + + shared_examples "export as" do |format| + it "should generally work" do + go(id: commit.id, format: format) + + expect(response).to be_success + end + + it "should generate it" do + expect_any_instance_of(Commit).to receive(:"to_#{format}") + + go(id: commit.id, format: format) + end + + it "should render it" do + go(id: commit.id, format: format) + + expect(response.body).to eq(commit.send(:"to_#{format}")) + end + + it "should not escape Html" do + allow_any_instance_of(Commit).to receive(:"to_#{format}"). + and_return('HTML entities &<>" ') + + go(id: commit.id, format: format) + + expect(response.body).not_to include('&') + expect(response.body).not_to include('>') + expect(response.body).not_to include('<') + expect(response.body).not_to include('"') + end + end + + describe "as diff" do + include_examples "export as", :diff + let(:format) { :diff } + + it "should really only be a git diff" do + go(id: commit.id, format: format) + + expect(response.body).to start_with("diff --git") + end + + it "should really only be a git diff without whitespace changes" do + go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format, w: 1) + + expect(response.body).to start_with("diff --git") + # without whitespace option, there are more than 2 diff_splits + diff_splits = assigns(:diffs).first.diff.split("\n") + expect(diff_splits.length).to be <= 2 + end + end + + describe "as patch" do + include_examples "export as", :patch + let(:format) { :patch } + + it "should really be a git email patch" do + go(id: commit.id, format: format) + + expect(response.body).to start_with("From #{commit.id}") + end + + it "should contain a git diff" do + go(id: commit.id, format: format) + + expect(response.body).to match(/^diff --git/) + end + end + + context 'commit that removes a submodule' do + render_views + + let(:fork_project) { create(:forked_project_with_submodules, visibility_level: 20) } + let(:commit) { fork_project.commit('remove-submodule') } + + it 'renders it' do + get(:show, + namespace_id: fork_project.namespace.to_param, + project_id: fork_project.to_param, + id: commit.id) + + expect(response).to be_success + end + end + end + + describe "GET branches" do + it "contains branch and tags information" do + get(:branches, namespace_id: project.namespace.to_param, project_id: project.to_param, - id: TestEnv::BRANCH_SHA['binary-encoding'], - format: "html") + id: commit.id) - expect(response).to be_success + expect(assigns(:branches)).to include("master", "feature_conflict") + expect(assigns(:tags)).to include("v1.1.0") + end + end + + describe 'POST revert' do + context 'when target branch is not provided' do + it 'should render the 404 page' do + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: commit.id) + + expect(response).not_to be_success + expect(response).to have_http_status(404) + end + end + + context 'when the revert was successful' do + it 'should redirect to the commits page' do + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: commit.id) + + expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master') + expect(flash[:notice]).to eq('The commit has been successfully reverted.') + end + end + + context 'when the revert failed' do + before do + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: commit.id) + end + + it 'should redirect to the commit page' do + # Reverting a commit that has been already reverted. + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: commit.id) + + expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, commit.id) + expect(flash[:alert]).to match('Sorry, we cannot revert this commit automatically.') + end + end + end + + describe 'POST cherry_pick' do + context 'when target branch is not provided' do + it 'should render the 404 page' do + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: master_pickable_commit.id) + + expect(response).not_to be_success + expect(response).to have_http_status(404) + end + end + + context 'when the cherry-pick was successful' do + it 'should redirect to the commits page' do + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: master_pickable_commit.id) + + expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master') + expect(flash[:notice]).to eq('The commit has been successfully cherry-picked.') + end end - def go(id:) - get :show, + context 'when the cherry_pick failed' do + before do + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: master_pickable_commit.id) + end + + it 'should redirect to the commit page' do + # Cherry-picking a commit that has been already cherry-picked. + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: master_pickable_commit.id) + + expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) + expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.') + end + end + end + + describe 'GET diff_for_path' do + def diff_for_path(extra_params = {}) + params = { namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: id + project_id: project.to_param + } + + get :diff_for_path, params.merge(extra_params) + end + + let(:existing_path) { '.gitmodules' } + + 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) + + expect(assigns(:diff_notes_disabled)).to be_falsey + expect(assigns(:comments_target)).to eq(noteable_type: 'Commit', + commit_id: commit.id) + end + + it 'only renders the diffs for the path given' do + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project| + expect(diffs.map(&:new_path)).to contain_exactly(existing_path) + meth.call(diffs, diff_refs, project) + end + + diff_for_path(id: commit.id, old_path: existing_path, new_path: existing_path) + end + end + + context 'when the path does not exist in the diff' do + before { diff_for_path(id: commit.id, old_path: existing_path.succ, new_path: existing_path.succ) } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'when the user does not have access to the project' do + before do + project.team.truncate + diff_for_path(id: commit.id, old_path: existing_path, new_path: existing_path) + end + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'when the commit does not exist' do + before { diff_for_path(id: commit.id.succ, old_path: existing_path, new_path: existing_path) } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end end end end diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 4018dac95a2..4058d5e2453 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -64,4 +64,73 @@ describe Projects::CompareController do expect(assigns(:commits)).to eq(nil) end end + + describe 'GET diff_for_path' do + def diff_for_path(extra_params = {}) + params = { + namespace_id: project.namespace.to_param, + project_id: project.to_param + } + + get :diff_for_path, params.merge(extra_params) + end + + let(:existing_path) { 'files/ruby/feature.rb' } + + context 'when the from and to refs exist' do + context 'when the user has access to the project' do + context 'when the path exists in the diff' do + it 'disables diff notes' do + diff_for_path(from: ref_from, to: ref_to, old_path: existing_path, new_path: existing_path) + + expect(assigns(:diff_notes_disabled)).to be_truthy + end + + it 'only renders the diffs for the path given' do + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project| + expect(diffs.map(&:new_path)).to contain_exactly(existing_path) + meth.call(diffs, diff_refs, project) + end + + diff_for_path(from: ref_from, to: ref_to, old_path: existing_path, new_path: existing_path) + end + end + + context 'when the path does not exist in the diff' do + before { diff_for_path(from: ref_from, to: ref_to, old_path: existing_path.succ, new_path: existing_path.succ) } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'when the user does not have access to the project' do + before do + project.team.truncate + diff_for_path(from: ref_from, to: ref_to, old_path: existing_path, new_path: existing_path) + end + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'when the from ref does not exist' do + before { diff_for_path(from: ref_from.succ, to: ref_to, old_path: existing_path, new_path: existing_path) } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + + context 'when the to ref does not exist' do + before { diff_for_path(from: ref_from, to: ref_to.succ, old_path: existing_path, new_path: existing_path) } + + it 'returns a 404' do + expect(response).to have_http_status(404) + 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 c4b57e77804..210085e3b1a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -10,7 +10,7 @@ describe Projects::MergeRequestsController do project.team << [user, :master] end - describe '#new' do + describe 'GET new' do context 'merge request that removes a submodule' do render_views @@ -34,7 +34,7 @@ describe Projects::MergeRequestsController do end end - describe "#show" do + describe "GET show" do shared_examples "export merge as" do |format| it "should generally work" do get(:show, @@ -108,7 +108,7 @@ describe Projects::MergeRequestsController do end end - describe 'GET #index' do + describe 'GET index' do def get_merge_requests get :index, namespace_id: project.namespace.to_param, @@ -140,7 +140,7 @@ describe Projects::MergeRequestsController do end end - describe 'PUT #update' do + describe 'PUT update' do context 'there is no source project' do let(:project) { create(:project) } let(:fork_project) { create(:forked_project_with_submodules) } @@ -168,7 +168,7 @@ describe Projects::MergeRequestsController do end end - describe 'POST #merge' do + describe 'POST merge' do let(:base_params) do { namespace_id: project.namespace.path, @@ -266,7 +266,7 @@ describe Projects::MergeRequestsController do end end - describe "DELETE #destroy" do + describe "DELETE destroy" do it "denies access to users unless they're admin or project owner" do delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid @@ -290,96 +290,210 @@ describe Projects::MergeRequestsController do end describe 'GET diffs' do - def go(format: 'html') - get :diffs, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: merge_request.iid, - format: format + def go(extra_params = {}) + params = { + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: merge_request.iid + } + + get :diffs, params.merge(extra_params) end - context 'as html' do - it 'renders the diff template' do - go + context 'with default params' do + context 'as html' do + before { go(format: 'html') } - expect(response).to render_template('diffs') + it 'renders the diff template' do + expect(response).to render_template('diffs') + end end - end - context 'as json' do - it 'renders the diffs template to a string' do - go format: 'json' + context 'as json' do + before { go(format: 'json') } - expect(response).to render_template('projects/merge_requests/show/_diffs') - expect(JSON.parse(response.body)).to have_key('html') + it 'renders the diffs template to a string' do + expect(response).to render_template('projects/merge_requests/show/_diffs') + expect(JSON.parse(response.body)).to have_key('html') + end end - end - - context 'with forked projects with submodules' do - render_views - let(:project) { create(:project) } - let(:fork_project) { create(:forked_project_with_submodules) } - let(:merge_request) { create(:merge_request_with_diffs, source_project: fork_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) } + context 'with forked projects with submodules' do + render_views - before do - fork_project.build_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id) - fork_project.save - merge_request.reload - end + let(:project) { create(:project) } + let(:fork_project) { create(:forked_project_with_submodules) } + let(:merge_request) { create(:merge_request_with_diffs, source_project: fork_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) } - it 'renders' do - go format: 'json' + before do + fork_project.build_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id) + fork_project.save + merge_request.reload + go(format: 'json') + end - expect(response).to be_success - expect(response.body).to have_content('Subproject commit') + it 'renders' do + expect(response).to be_success + expect(response.body).to have_content('Subproject commit') + end end end - end - describe 'GET diffs with ignore_whitespace_change' do - def go(format: 'html') - get :diffs, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: merge_request.iid, - format: format, - w: 1 - end + context 'with ignore_whitespace_change' do + context 'as html' do + before { go(format: 'html', w: 1) } - context 'as html' do - it 'renders the diff template' do - go + it 'renders the diff template' do + expect(response).to render_template('diffs') + end + end + + context 'as json' do + before { go(format: 'json', w: 1) } - expect(response).to render_template('diffs') + it 'renders the diffs template to a string' do + expect(response).to render_template('projects/merge_requests/show/_diffs') + expect(JSON.parse(response.body)).to have_key('html') + end end end - context 'as json' do - it 'renders the diffs template to a string' do - go format: 'json' + context 'with view' do + before { go(view: 'parallel') } - expect(response).to render_template('projects/merge_requests/show/_diffs') - expect(JSON.parse(response.body)).to have_key('html') + it 'saves the preferred diff view in a cookie' do + expect(response.cookies['diff_view']).to eq('parallel') end end end - describe 'GET diffs with view' do - def go(extra_params = {}) + describe 'GET diff_for_path' do + def diff_for_path(extra_params = {}) params = { namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: merge_request.iid + project_id: project.to_param } - get :diffs, params.merge(extra_params) + get :diff_for_path, params.merge(extra_params) end - it 'saves the preferred diff view in a cookie' do - go view: 'parallel' + context 'when an ID param is passed' do + let(:existing_path) { 'files/ruby/popen.rb' } - expect(response.cookies['diff_view']).to eq('parallel') + context 'when the merge request exists' do + context 'when the user can view the merge request' do + context 'when the path exists in the diff' do + it 'enables diff notes' do + diff_for_path(id: merge_request.iid, old_path: existing_path, new_path: existing_path) + + expect(assigns(:diff_notes_disabled)).to be_falsey + expect(assigns(:comments_target)).to eq(noteable_type: 'MergeRequest', + noteable_id: merge_request.id) + end + + it 'only renders the diffs for the path given' do + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project| + expect(diffs.map(&:new_path)).to contain_exactly(existing_path) + meth.call(diffs, diff_refs, project) + end + + diff_for_path(id: merge_request.iid, old_path: existing_path, new_path: existing_path) + end + end + + context 'when the path does not exist in the diff' do + before { diff_for_path(id: merge_request.iid, old_path: 'files/ruby/nopen.rb', new_path: 'files/ruby/nopen.rb') } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'when the user cannot view the merge request' do + before do + project.team.truncate + diff_for_path(id: merge_request.iid, old_path: existing_path, new_path: existing_path) + end + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'when the merge request does not exist' do + before { diff_for_path(id: merge_request.iid.succ, old_path: existing_path, new_path: existing_path) } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + + context 'when the merge request belongs to a different project' do + let(:other_project) { create(:empty_project) } + + before do + other_project.team << [user, :master] + diff_for_path(id: merge_request.iid, old_path: existing_path, new_path: existing_path, project_id: other_project.to_param) + end + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'when source and target params are passed' do + let(:existing_path) { 'files/ruby/feature.rb' } + + context 'when both branches are in the same project' do + it 'disables diff notes' do + diff_for_path(old_path: existing_path, new_path: existing_path, merge_request: { source_branch: 'feature', target_branch: 'master' }) + + expect(assigns(:diff_notes_disabled)).to be_truthy + end + + it 'only renders the diffs for the path given' do + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project| + expect(diffs.map(&:new_path)).to contain_exactly(existing_path) + meth.call(diffs, diff_refs, project) + end + + diff_for_path(old_path: existing_path, new_path: existing_path, merge_request: { source_branch: 'feature', target_branch: 'master' }) + end + end + + context 'when the source branch is in a different project to the target' do + let(:other_project) { create(:project) } + + before { other_project.team << [user, :master] } + + context 'when the path exists in the diff' do + it 'disables diff notes' do + diff_for_path(old_path: existing_path, new_path: existing_path, merge_request: { source_project: other_project, source_branch: 'feature', target_branch: 'master' }) + + expect(assigns(:diff_notes_disabled)).to be_truthy + end + + it 'only renders the diffs for the path given' do + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project| + expect(diffs.map(&:new_path)).to contain_exactly(existing_path) + meth.call(diffs, diff_refs, project) + end + + diff_for_path(old_path: existing_path, new_path: existing_path, merge_request: { source_project: other_project, source_branch: 'feature', target_branch: 'master' }) + end + end + + context 'when the path does not exist in the diff' do + before { diff_for_path(old_path: 'files/ruby/nopen.rb', new_path: 'files/ruby/nopen.rb', merge_request: { source_project: other_project, source_branch: 'feature', target_branch: 'master' }) } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 5c8ddbebf0d..b682ced75ac 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -2,7 +2,7 @@ FactoryGirl.define do # Project without repository # # Project does not have bare repository. - # Use this factory if you dont need repository in tests + # Use this factory if you don't need repository in tests factory :empty_project, class: 'Project' do sequence(:name) { |n| "project#{n}" } path { name.downcase.gsub(/\s/, '_') } diff --git a/spec/features/admin/admin_abuse_reports_spec.rb b/spec/features/admin/admin_abuse_reports_spec.rb new file mode 100644 index 00000000000..16baf7e9516 --- /dev/null +++ b/spec/features/admin/admin_abuse_reports_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe "Admin::AbuseReports", feature: true, js: true do + let(:user) { create(:user) } + + context 'as an admin' do + describe 'if a user has been reported for abuse' do + before do + create(:abuse_report, user: user) + login_as :admin + end + + describe 'in the abuse report view' do + it "should present a link to the user's profile" do + visit admin_abuse_reports_path + + expect(page).to have_link user.name, href: user_path(user) + end + end + + describe 'in the profile page of the user' do + it 'should show a link to the admin view of the user' do + visit user_path(user) + + expect(page).to have_link '', href: admin_user_path(user) + end + end + end + end +end diff --git a/spec/features/expand_collapse_diffs_spec.rb b/spec/features/expand_collapse_diffs_spec.rb new file mode 100644 index 00000000000..7cff196c8d9 --- /dev/null +++ b/spec/features/expand_collapse_diffs_spec.rb @@ -0,0 +1,207 @@ +require 'spec_helper' + +feature 'Expand and collapse diffs', js: true, feature: true do + include WaitForAjax + + before do + login_as :admin + project = create(:project) + branch = 'expand-collapse-diffs' + + # Ensure that undiffable.md is in .gitattributes + project.repository.copy_gitattributes(branch) + visit namespace_project_commit_path(project.namespace, project, project.commit(branch)) + execute_script('window.ajaxUris = []; $(document).ajaxSend(function(event, xhr, settings) { ajaxUris.push(settings.url) });') + end + + def file_container(filename) + find("[data-blob-diff-path*='#{filename}']") + end + + # Use define_method instead of let (which is memoized) so that this just works across a + # reload. + # + files = [ + 'small_diff.md', 'large_diff.md', 'large_diff_renamed.md', 'undiffable.md', + 'too_large.md', 'too_large_image.jpg' + ] + + files.each do |file| + define_method(file.split('.').first) { file_container(file) } + end + + context 'visiting a commit with collapsed diffs' do + it 'shows small diffs immediately' do + expect(small_diff).to have_selector('.code') + expect(small_diff).not_to have_selector('.nothing-here-block') + end + + it 'collapses large diffs by default' do + expect(large_diff).not_to have_selector('.code') + expect(large_diff).to have_selector('.nothing-here-block') + end + + it 'collapses large diffs for renamed files by default' do + expect(large_diff_renamed).not_to have_selector('.code') + expect(large_diff_renamed).to have_selector('.nothing-here-block') + expect(large_diff_renamed).to have_selector('.file-title .deletion') + expect(large_diff_renamed).to have_selector('.file-title .addition') + end + + it 'shows non-renderable diffs as such immediately, regardless of their size' do + expect(undiffable).not_to have_selector('.code') + expect(undiffable).to have_selector('.nothing-here-block') + expect(undiffable).to have_content('gitattributes') + end + + it 'does not allow diffs that are larger than the maximum size to be expanded' do + expect(too_large).not_to have_selector('.code') + expect(too_large).to have_selector('.nothing-here-block') + expect(too_large).to have_content('too large') + end + + it 'shows image diffs immediately, regardless of their size' do + expect(too_large_image).not_to have_selector('.nothing-here-block') + expect(too_large_image).to have_selector('.image') + end + + context 'expanding a diff for a renamed file' do + before do + large_diff_renamed.find('.nothing-here-block').click + wait_for_ajax + end + + it 'shows the old content' do + old_line = large_diff_renamed.find('.line_content.old') + + expect(old_line).to have_content('two copies') + end + + it 'shows the new content' do + new_line = large_diff_renamed.find('.line_content.new', match: :prefer_exact) + + expect(new_line).to have_content('three copies') + end + end + + context 'expanding a large diff' do + before do + click_link('large_diff.md') + wait_for_ajax + end + + it 'makes a request to get the content' do + ajax_uris = evaluate_script('ajaxUris') + + expect(ajax_uris).not_to be_empty + expect(ajax_uris.first).to include('large_diff.md') + end + + it 'shows the diff content' do + expect(large_diff).to have_selector('.code') + expect(large_diff).not_to have_selector('.nothing-here-block') + end + + context 'adding a comment to the expanded diff' do + let(:comment_text) { 'A comment' } + + before do + large_diff.find('.line_holder', match: :prefer_exact).hover + large_diff.find('.add-diff-note').click + large_diff.find('.note-textarea').send_keys comment_text + large_diff.find_button('Comment').click + wait_for_ajax + end + + it 'adds the comment' do + expect(large_diff.find('.notes')).to have_content comment_text + end + + context 'reloading the page' do + before { refresh } + + it 'collapses the large diff by default' do + expect(large_diff).not_to have_selector('.code') + expect(large_diff).to have_selector('.nothing-here-block') + end + + context 'expanding the diff' do + before do + click_link('large_diff.md') + wait_for_ajax + end + + it 'shows the diff content' do + expect(large_diff).to have_selector('.code') + expect(large_diff).not_to have_selector('.nothing-here-block') + end + + it 'shows the diff comment' do + expect(large_diff.find('.notes')).to have_content comment_text + end + end + end + end + end + + context 'collapsing an expanded diff' do + before { click_link('small_diff.md') } + + it 'hides the diff content' do + expect(small_diff).not_to have_selector('.code') + expect(small_diff).to have_selector('.nothing-here-block') + end + + context 're-expanding the same diff' do + before { click_link('small_diff.md') } + + it 'shows the diff content' do + expect(small_diff).to have_selector('.code') + expect(small_diff).not_to have_selector('.nothing-here-block') + end + + it 'does not make a new HTTP request' do + expect(evaluate_script('ajaxUris')).to be_empty + end + end + end + end + + context 'expanding all diffs' do + before do + click_link('Expand all') + wait_for_ajax + execute_script('window.ajaxUris = []; $(document).ajaxSend(function(event, xhr, settings) { ajaxUris.push(settings.url) });') + end + + it 'reloads the page with all diffs expanded' do + expect(small_diff).to have_selector('.code') + expect(small_diff).not_to have_selector('.nothing-here-block') + + expect(large_diff).to have_selector('.code') + expect(large_diff).not_to have_selector('.nothing-here-block') + end + + context 'collapsing an expanded diff' do + before { click_link('small_diff.md') } + + it 'hides the diff content' do + expect(small_diff).not_to have_selector('.code') + expect(small_diff).to have_selector('.nothing-here-block') + end + + context 're-expanding the same diff' do + before { click_link('small_diff.md') } + + it 'shows the diff content' do + expect(small_diff).to have_selector('.code') + expect(small_diff).not_to have_selector('.nothing-here-block') + end + + it 'does not make a new HTTP request' do + expect(evaluate_script('ajaxUris')).to be_empty + end + end + end + end +end diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 006a06b8235..4b9b5394b61 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -7,6 +7,7 @@ describe 'Filter issues', feature: true do let!(:user) { create(:user)} let!(:milestone) { create(:milestone, project: project) } let!(:label) { create(:label, project: project) } + let!(:issue1) { create(:issue, project: project) } before do project.team << [user, :master] @@ -196,6 +197,7 @@ describe 'Filter issues', feature: true do page.within '.labels-filter' do click_link 'bug' end + find('.dropdown-menu-close-icon').click page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) @@ -287,7 +289,7 @@ describe 'Filter issues', feature: true do wait_for_ajax page.within '.issues-list' do - expect(first('.issue')).to have_content('Frontend') + expect(page).to have_content('Frontend') end end end diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index 6a39c302f55..98ba93b4036 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -76,7 +76,7 @@ feature 'Prioritize labels', feature: true do expect(page.all('li').last).to have_content('bug') end - visit current_url + refresh wait_for_ajax page.within('.prioritized-labels') do diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb new file mode 100644 index 00000000000..d94dee0c797 --- /dev/null +++ b/spec/features/protected_branches_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +feature 'Projected Branches', feature: true, js: true do + let(:user) { create(:user, :admin) } + let(:project) { create(:project) } + + before { login_as(user) } + + def set_protected_branch_name(branch_name) + find(".js-protected-branch-select").click + find(".dropdown-input-field").set(branch_name) + click_on "Create Protected Branch: #{branch_name}" + end + + describe "explicit protected branches" do + it "allows creating explicit protected branches" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('some-branch') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content('some-branch') } + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.name).to eq('some-branch') + end + + it "displays the last commit on the matching branch if it exists" do + commit = create(:commit, project: project) + project.repository.add_branch(user, 'some-branch', commit.id) + + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('some-branch') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content(commit.id[0..7]) } + end + + it "displays an error message if the named branch does not exist" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('some-branch') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content('branch was removed') } + end + end + + describe "wildcard protected branches" do + it "allows creating protected branches with a wildcard" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('*-stable') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content('*-stable') } + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.name).to eq('*-stable') + end + + it "displays the number of matching branches" do + project.repository.add_branch(user, 'production-stable', 'master') + project.repository.add_branch(user, 'staging-stable', 'master') + + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('*-stable') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content("2 matching branches") } + end + + it "displays all the branches matching the wildcard" do + project.repository.add_branch(user, 'production-stable', 'master') + project.repository.add_branch(user, 'staging-stable', 'master') + project.repository.add_branch(user, 'development', 'master') + create(:protected_branch, project: project, name: "*-stable") + + visit namespace_project_protected_branches_path(project.namespace, project) + click_on "2 matching branches" + + within(".protected-branches-list") do + expect(page).to have_content("production-stable") + expect(page).to have_content("staging-stable") + expect(page).not_to have_content("development") + end + end + end +end diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index 85923f0a19d..d0a301038c4 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe "Search", feature: true do let(:user) { create(:user) } let(:project) { create(:project, namespace: user.namespace) } + let!(:issue) { create(:issue, project: project, assignee: user) } + let!(:issue2) { create(:issue, project: project, author: user) } before do login_with(user) diff --git a/spec/fixtures/blockquote_fence_after.md b/spec/fixtures/blockquote_fence_after.md new file mode 100644 index 00000000000..2652a842c0e --- /dev/null +++ b/spec/fixtures/blockquote_fence_after.md @@ -0,0 +1,115 @@ +Single `>>>` inside code block: + +``` +# Code +>>> +# Code +``` + +Double `>>>` inside code block: + +```txt +# Code +>>> +# Code +>>> +# Code +``` + +Blockquote outside code block: + +> Quote + +Code block inside blockquote: + +> Quote +> +> ``` +> # Code +> ``` +> +> Quote + +Single `>>>` inside code block inside blockquote: + +> Quote +> +> ``` +> # Code +> >>> +> # Code +> ``` +> +> Quote + +Double `>>>` inside code block inside blockquote: + +> Quote +> +> ``` +> # Code +> >>> +> # Code +> >>> +> # Code +> ``` +> +> Quote + +Single `>>>` inside HTML: + +<pre> +# Code +>>> +# Code +</pre> + +Double `>>>` inside HTML: + +<pre> +# Code +>>> +# Code +>>> +# Code +</pre> + +Blockquote outside HTML: + +> Quote + +HTML inside blockquote: + +> Quote +> +> <pre> +> # Code +> </pre> +> +> Quote + +Single `>>>` inside HTML inside blockquote: + +> Quote +> +> <pre> +> # Code +> >>> +> # Code +> </pre> +> +> Quote + +Double `>>>` inside HTML inside blockquote: + +> Quote +> +> <pre> +> # Code +> >>> +> # Code +> >>> +> # Code +> </pre> +> +> Quote diff --git a/spec/fixtures/blockquote_fence_before.md b/spec/fixtures/blockquote_fence_before.md new file mode 100644 index 00000000000..d52eec72896 --- /dev/null +++ b/spec/fixtures/blockquote_fence_before.md @@ -0,0 +1,131 @@ +Single `>>>` inside code block: + +``` +# Code +>>> +# Code +``` + +Double `>>>` inside code block: + +```txt +# Code +>>> +# Code +>>> +# Code +``` + +Blockquote outside code block: + +>>> +Quote +>>> + +Code block inside blockquote: + +>>> +Quote + +``` +# Code +``` + +Quote +>>> + +Single `>>>` inside code block inside blockquote: + +>>> +Quote + +``` +# Code +>>> +# Code +``` + +Quote +>>> + +Double `>>>` inside code block inside blockquote: + +>>> +Quote + +``` +# Code +>>> +# Code +>>> +# Code +``` + +Quote +>>> + +Single `>>>` inside HTML: + +<pre> +# Code +>>> +# Code +</pre> + +Double `>>>` inside HTML: + +<pre> +# Code +>>> +# Code +>>> +# Code +</pre> + +Blockquote outside HTML: + +>>> +Quote +>>> + +HTML inside blockquote: + +>>> +Quote + +<pre> +# Code +</pre> + +Quote +>>> + +Single `>>>` inside HTML inside blockquote: + +>>> +Quote + +<pre> +# Code +>>> +# Code +</pre> + +Quote +>>> + +Double `>>>` inside HTML inside blockquote: + +>>> +Quote + +<pre> +# Code +>>> +# Code +>>> +# Code +</pre> + +Quote +>>> diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 7ea9fa933f0..4b134a48410 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -31,26 +31,11 @@ describe DiffHelper do end end - describe 'diff_hard_limit_enabled?' do - it 'should return true if param is provided' do - allow(controller).to receive(:params) { { force_show_diff: true } } - expect(diff_hard_limit_enabled?).to be_truthy - end - - it 'should return false if param is not provided' do - expect(diff_hard_limit_enabled?).to be_falsey - end - end - describe 'diff_options' do - it 'should return hard limit for a diff if force diff is true' do + it 'should return hard limit for a diff' do allow(controller).to receive(:params) { { force_show_diff: true } } expect(diff_options).to include(Commit.max_diff_options) end - - it 'should return safe limit for a diff if force diff is false' do - expect(diff_options).not_to include(:max_lines, :max_files) - end end describe 'unfold_bottom_class' do diff --git a/spec/javascripts/fixtures/issues_show.html.haml b/spec/javascripts/fixtures/issues_show.html.haml index 470cabeafbb..06c2ab1e823 100644 --- a/spec/javascripts/fixtures/issues_show.html.haml +++ b/spec/javascripts/fixtures/issues_show.html.haml @@ -1,7 +1,7 @@ :css .hidden { display: none !important; } -.flash-container +.flash-container.flash-container-page .flash-alert .flash-notice diff --git a/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb b/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb new file mode 100644 index 00000000000..2799249ae3e --- /dev/null +++ b/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb @@ -0,0 +1,14 @@ +require 'rails_helper' + +describe Banzai::Filter::BlockquoteFenceFilter, lib: true do + include FilterSpecHelper + + it 'converts blockquote fences to blockquote lines' do + content = File.read(Rails.root.join('spec/fixtures/blockquote_fence_before.md')) + expected = File.read(Rails.root.join('spec/fixtures/blockquote_fence_after.md')) + + output = filter(content) + + expect(output).to eq(expected) + end +end diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb index 543b4786d84..ac9c66e2663 100644 --- a/spec/lib/banzai/reference_parser/base_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -234,4 +234,79 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do to eq([project]) end end + + describe '#collection_objects_for_ids' do + context 'with RequestStore disabled' do + it 'queries the collection directly' do + collection = User.all + + expect(collection).to receive(:where).twice.and_call_original + + 2.times do + expect(subject.collection_objects_for_ids(collection, [user.id])). + to eq([user]) + end + end + end + + context 'with RequestStore enabled' do + before do + cache = Hash.new { |hash, key| hash[key] = {} } + + allow(RequestStore).to receive(:active?).and_return(true) + allow(subject).to receive(:collection_cache).and_return(cache) + end + + it 'queries the collection on the first call' do + expect(subject.collection_objects_for_ids(User, [user.id])). + to eq([user]) + end + + it 'does not query previously queried objects' do + collection = User.all + + expect(collection).to receive(:where).once.and_call_original + + 2.times do + expect(subject.collection_objects_for_ids(collection, [user.id])). + to eq([user]) + end + end + + it 'casts String based IDs to Fixnums before querying objects' do + 2.times do + expect(subject.collection_objects_for_ids(User, [user.id.to_s])). + to eq([user]) + end + end + + it 'queries any additional objects after the first call' do + other_user = create(:user) + + expect(subject.collection_objects_for_ids(User, [user.id])). + to eq([user]) + + expect(subject.collection_objects_for_ids(User, [user.id, other_user.id])). + to eq([user, other_user]) + end + + it 'caches objects on a per collection class basis' do + expect(subject.collection_objects_for_ids(User, [user.id])). + to eq([user]) + + expect(subject.collection_objects_for_ids(Project, [project.id])). + to eq([project]) + end + end + end + + describe '#collection_cache_key' do + it 'returns the cache key for a Class' do + expect(subject.collection_cache_key(Project)).to eq(Project) + end + + it 'returns the cache key for an ActiveRecord::Relation' do + expect(subject.collection_cache_key(Project.all)).to eq(Project) + end + end end diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb new file mode 100644 index 00000000000..a15aa173fbd --- /dev/null +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' +require 'fileutils' + +describe Gitlab::Git::Hook, lib: true do + describe "#trigger" do + let(:project) { create(:project) } + let(:user) { create(:user) } + + def create_hook(name) + FileUtils.mkdir_p(File.join(project.repository.path, 'hooks')) + File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f| + f.write('exit 0') + end + end + + def create_failing_hook(name) + FileUtils.mkdir_p(File.join(project.repository.path, 'hooks')) + File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f| + f.write(<<-HOOK) + echo 'regular message from the hook' + echo 'error message from the hook' 1>&2 + exit 1 + HOOK + end + end + + ['pre-receive', 'post-receive', 'update'].each do |hook_name| + + context "when triggering a #{hook_name} hook" do + context "when the hook is successful" do + it "returns success with no errors" do + create_hook(hook_name) + hook = Gitlab::Git::Hook.new(hook_name, project.repository.path) + blank = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' + + status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref) + expect(status).to be true + expect(errors).to be_blank + end + end + + context "when the hook is unsuccessful" do + it "returns failure with errors" do + create_failing_hook(hook_name) + hook = Gitlab::Git::Hook.new(hook_name, project.repository.path) + blank = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' + + status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref) + expect(status).to be false + expect(errors).to eq("error message from the hook\n") + end + end + end + end + + context "when the hook doesn't exist" do + it "returns success with no errors" do + hook = Gitlab::Git::Hook.new('unknown_hook', project.repository.path) + blank = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' + + status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref) + expect(status).to be true + expect(errors).to be_nil + end + end + end +end diff --git a/spec/lib/gitlab/graphs/commits_spec.rb b/spec/lib/gitlab/graphs/commits_spec.rb new file mode 100644 index 00000000000..f5c064303ad --- /dev/null +++ b/spec/lib/gitlab/graphs/commits_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Gitlab::Graphs::Commits, lib: true do + let!(:project) { create(:project, :public, :empty_repo) } + + let!(:commit1) { create(:commit, git_commit: RepoHelpers.sample_commit, project: project, committed_date: Time.now) } + let!(:commit1_yesterday) { create(:commit, git_commit: RepoHelpers.sample_commit, project: project, committed_date: 1.day.ago)} + + let!(:commit2) { create(:commit, git_commit: RepoHelpers.another_sample_commit, project: project, committed_date: Time.now) } + + describe '#commit_per_day' do + context 'when range is only commits from today' do + subject { described_class.new([commit2, commit1]).commit_per_day } + it { is_expected.to eq 2 } + end + end + + context 'when range is only commits from today' do + subject { described_class.new([commit2, commit1]) } + describe '#commit_per_day' do + it { expect(subject.commit_per_day).to eq 2 } + end + + describe '#duration' do + it { expect(subject.duration).to eq 0 } + end + end + + context 'with commits from yesterday and today' do + subject { described_class.new([commit2, commit1_yesterday]) } + describe '#commit_per_day' do + it { expect(subject.commit_per_day).to eq 1 } + end + + describe '#duration' do + it { expect(subject.duration).to eq 1 } + end + end +end diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 0b30e8c9b04..7286b0c39c0 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -4208,7 +4208,18 @@ "name": "User 4" }, "events": [ - + { + "id": 529, + "target_type": "Note", + "target_id": 2521, + "title": "test levels", + "data": null, + "project_id": 4, + "created_at": "2016-07-07T14:35:12.128Z", + "updated_at": "2016-07-07T14:35:12.128Z", + "action": 6, + "author_id": 1 + } ] }, { diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index a72aaa44e82..05ffec8ea0a 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -24,11 +24,27 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do expect(Ci::Pipeline.first.notes).not_to be_empty end - it 'restores the correct event' do + it 'restores the correct event with symbolised data' do restored_project_json expect(Event.where.not(data: nil).first.data[:ref]).not_to be_empty end + + context 'event at forth level of the tree' do + let(:event) { Event.where(title: 'test levels').first } + + before do + restored_project_json + end + + it 'restores the event' do + expect(event).not_to be_nil + end + + it 'event belongs to note, belongs to merge request, belongs to a project' do + expect(event.note.noteable.project).not_to be_nil + end + end end end end diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 59024d3290b..2cb74629da8 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -45,6 +45,12 @@ describe Gitlab::UrlSanitizer, lib: true do expect(filtered_content).to include("user@server:project.git") end + + it 'returns an empty string for invalid URLs' do + filtered_content = sanitize_url('ssh://') + + expect(filtered_content).to include("repository '' not found") + end end describe '#sanitized_url' do diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb new file mode 100644 index 00000000000..9a637c94fbe --- /dev/null +++ b/spec/models/merge_request_diff_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe MergeRequestDiff, models: true do + describe '#diffs' do + let(:mr) { create(:merge_request, :with_diffs) } + let(:mr_diff) { mr.merge_request_diff } + + context 'when the :ignore_whitespace_change option is set' do + it 'creates a new compare object instead of loading from the DB' do + expect(mr_diff).not_to receive(:load_diffs) + expect(Gitlab::Git::Compare).to receive(:new).and_call_original + + mr_diff.diffs(ignore_whitespace_change: true) + end + end + + context 'when the raw diffs are empty' do + before { mr_diff.update_attributes(st_diffs: '') } + + it 'returns an empty DiffCollection' do + expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection) + expect(mr_diff.diffs).to be_empty + end + end + + context 'when the raw diffs exist' do + it 'returns the diffs' do + expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection) + expect(mr_diff.diffs).not_to be_empty + end + + context 'when the :paths option is set' do + let(:diffs) { mr_diff.diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) } + + it 'only returns diffs that match the (old path, new path) given' do + expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb') + end + + it 'uses the diffs from the DB' do + expect(mr_diff).to receive(:load_diffs) + + diffs + end + end + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a4b6ff8f8ad..c8ad7ab3e7f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -116,6 +116,31 @@ describe MergeRequest, models: true do end end + describe '#diffs' do + let(:merge_request) { build(:merge_request) } + let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } } + + context 'when there are MR diffs' do + it 'delegates to the MR diffs' do + merge_request.merge_request_diff = MergeRequestDiff.new + + expect(merge_request.merge_request_diff).to receive(:diffs).with(options) + + merge_request.diffs(options) + end + end + + context 'when there are no MR diffs' do + it 'delegates to the compare object' do + merge_request.compare = double(:compare) + + expect(merge_request.compare).to receive(:diffs).with(options) + + merge_request.diffs(options) + end + end + end + describe "#mr_and_commit_notes" do let!(:merge_request) { create(:merge_request) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1b434a726dc..5a27ccbab0a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -449,6 +449,14 @@ describe Project, models: true do it { expect(project.open_branches.map(&:name)).to include('feature') } it { expect(project.open_branches.map(&:name)).not_to include('master') } + + it "includes branches matching a protected branch wildcard" do + expect(project.open_branches.map(&:name)).to include('feature') + + create(:protected_branch, name: 'feat*', project: project) + + expect(Project.find(project.id).open_branches.map(&:name)).to include('feature') + end end describe '#star_count' do @@ -949,15 +957,67 @@ describe Project, models: true do describe '#protected_branch?' do let(:project) { create(:empty_project) } - it 'returns true when a branch is a protected branch' do + it 'returns true when the branch matches a protected branch via direct match' do project.protected_branches.create!(name: 'foo') expect(project.protected_branch?('foo')).to eq(true) end - it 'returns false when a branch is not a protected branch' do + it 'returns true when the branch matches a protected branch via wildcard match' do + project.protected_branches.create!(name: 'production/*') + + expect(project.protected_branch?('production/some-branch')).to eq(true) + end + + it 'returns false when the branch does not match a protected branch via direct match' do expect(project.protected_branch?('foo')).to eq(false) end + + it 'returns false when the branch does not match a protected branch via wildcard match' do + project.protected_branches.create!(name: 'production/*') + + expect(project.protected_branch?('staging/some-branch')).to eq(false) + end + end + + describe "#developers_can_push_to_protected_branch?" do + let(:project) { create(:empty_project) } + + context "when the branch matches a protected branch via direct match" do + it "returns true if 'Developers can Push' is turned on" do + create(:protected_branch, name: "production", project: project, developers_can_push: true) + + expect(project.developers_can_push_to_protected_branch?('production')).to be true + end + + it "returns false if 'Developers can Push' is turned off" do + create(:protected_branch, name: "production", project: project, developers_can_push: false) + + expect(project.developers_can_push_to_protected_branch?('production')).to be false + end + end + + context "when the branch matches a protected branch via wilcard match" do + it "returns true if 'Developers can Push' is turned on" do + create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + + expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true + end + + it "returns false if 'Developers can Push' is turned off" do + create(:protected_branch, name: "production/*", project: project, developers_can_push: false) + + expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false + end + end + + context "when the branch does not match a protected branch" do + it "returns false" do + create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + + expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false + end + end end describe '#container_registry_path_with_namespace' do diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index b523834c6e9..8bf0d24a128 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe ProtectedBranch, models: true do + subject { build_stubbed(:protected_branch) } + describe 'Associations' do it { is_expected.to belong_to(:project) } end @@ -12,4 +14,127 @@ describe ProtectedBranch, models: true do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:name) } end + + describe "#matches?" do + context "when the protected branch setting is not a wildcard" do + let(:protected_branch) { build(:protected_branch, name: "production/some-branch") } + + it "returns true for branch names that are an exact match" do + expect(protected_branch.matches?("production/some-branch")).to be true + end + + it "returns false for branch names that are not an exact match" do + expect(protected_branch.matches?("staging/some-branch")).to be false + end + end + + context "when the protected branch name contains wildcard(s)" do + context "when there is a single '*'" do + let(:protected_branch) { build(:protected_branch, name: "production/*") } + + it "returns true for branch names matching the wildcard" do + expect(protected_branch.matches?("production/some-branch")).to be true + expect(protected_branch.matches?("production/")).to be true + end + + it "returns false for branch names not matching the wildcard" do + expect(protected_branch.matches?("staging/some-branch")).to be false + expect(protected_branch.matches?("production")).to be false + end + end + + context "when the wildcard contains regex symbols other than a '*'" do + let(:protected_branch) { build(:protected_branch, name: "pro.duc.tion/*") } + + it "returns true for branch names matching the wildcard" do + expect(protected_branch.matches?("pro.duc.tion/some-branch")).to be true + end + + it "returns false for branch names not matching the wildcard" do + expect(protected_branch.matches?("production/some-branch")).to be false + expect(protected_branch.matches?("proXducYtion/some-branch")).to be false + end + end + + context "when there are '*'s at either end" do + let(:protected_branch) { build(:protected_branch, name: "*/production/*") } + + it "returns true for branch names matching the wildcard" do + expect(protected_branch.matches?("gitlab/production/some-branch")).to be true + expect(protected_branch.matches?("/production/some-branch")).to be true + expect(protected_branch.matches?("gitlab/production/")).to be true + expect(protected_branch.matches?("/production/")).to be true + end + + it "returns false for branch names not matching the wildcard" do + expect(protected_branch.matches?("gitlabproductionsome-branch")).to be false + expect(protected_branch.matches?("production/some-branch")).to be false + expect(protected_branch.matches?("gitlab/production")).to be false + expect(protected_branch.matches?("production")).to be false + end + end + + context "when there are arbitrarily placed '*'s" do + let(:protected_branch) { build(:protected_branch, name: "pro*duction/*/gitlab/*") } + + it "returns true for branch names matching the wildcard" do + expect(protected_branch.matches?("production/some-branch/gitlab/second-branch")).to be true + expect(protected_branch.matches?("proXYZduction/some-branch/gitlab/second-branch")).to be true + expect(protected_branch.matches?("proXYZduction/gitlab/gitlab/gitlab")).to be true + expect(protected_branch.matches?("proXYZduction//gitlab/")).to be true + expect(protected_branch.matches?("proXYZduction/some-branch/gitlab/")).to be true + expect(protected_branch.matches?("proXYZduction//gitlab/some-branch")).to be true + end + + it "returns false for branch names not matching the wildcard" do + expect(protected_branch.matches?("production/some-branch/not-gitlab/second-branch")).to be false + expect(protected_branch.matches?("prodXYZuction/some-branch/gitlab/second-branch")).to be false + expect(protected_branch.matches?("proXYZduction/gitlab/some-branch/gitlab")).to be false + expect(protected_branch.matches?("proXYZduction/gitlab//")).to be false + expect(protected_branch.matches?("proXYZduction/gitlab/")).to be false + expect(protected_branch.matches?("proXYZduction//some-branch/gitlab")).to be false + end + end + end + end + + describe "#matching" do + context "for direct matches" do + it "returns a list of protected branches matching the given branch name" do + production = create(:protected_branch, name: "production") + staging = create(:protected_branch, name: "staging") + + expect(ProtectedBranch.matching("production")).to include(production) + expect(ProtectedBranch.matching("production")).not_to include(staging) + end + + it "accepts a list of protected branches to search from, so as to avoid a DB call" do + production = build(:protected_branch, name: "production") + staging = build(:protected_branch, name: "staging") + + expect(ProtectedBranch.matching("production")).to be_empty + expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).to include(production) + expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).not_to include(staging) + end + end + + context "for wildcard matches" do + it "returns a list of protected branches matching the given branch name" do + production = create(:protected_branch, name: "production/*") + staging = create(:protected_branch, name: "staging/*") + + expect(ProtectedBranch.matching("production/some-branch")).to include(production) + expect(ProtectedBranch.matching("production/some-branch")).not_to include(staging) + end + + it "accepts a list of protected branches to search from, so as to avoid a DB call" do + production = build(:protected_branch, name: "production/*") + staging = build(:protected_branch, name: "staging/*") + + expect(ProtectedBranch.matching("production/some-branch")).to be_empty + expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).to include(production) + expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).not_to include(staging) + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 328254ed56b..3984b30ddf8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -446,6 +446,7 @@ describe User, models: true do it { expect(user.can_create_group?).to be_truthy } it { expect(user.can_create_project?).to be_truthy } it { expect(user.first_name).to eq('John') } + it { expect(user.external).to be_falsey } end describe 'with defaults' do @@ -468,6 +469,26 @@ describe User, models: true do expect(user.theme_id).to eq(1) end end + + context 'when current_application_settings.user_default_external is true' do + before do + stub_application_setting(user_default_external: true) + end + + it "creates external user by default" do + user = build(:user) + + expect(user.external).to be_truthy + end + + describe 'with default overrides' do + it "creates a non-external user" do + user = build(:user, external: false) + + expect(user.external).to be_falsey + end + end + end end describe '.find_by_any_email' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 04141a45031..c2c94040ece 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -49,10 +49,25 @@ describe API::API, api: true do describe "GET /groups/:id" do context "when authenticated as user" do - it "should return one of user1's groups" do + it "returns one of user1's groups" do + project = create(:project, namespace: group2, path: 'Foo') + create(:project_group_link, project: project, group: group1) + get api("/groups/#{group1.id}", user1) + expect(response).to have_http_status(200) - json_response['name'] == group1.name + expect(json_response['id']).to eq(group1.id) + expect(json_response['name']).to eq(group1.name) + expect(json_response['path']).to eq(group1.path) + expect(json_response['description']).to eq(group1.description) + expect(json_response['visibility_level']).to eq(group1.visibility_level) + expect(json_response['avatar_url']).to eq(group1.avatar_url) + expect(json_response['web_url']).to eq(group1.web_url) + expect(json_response['projects']).to be_an Array + expect(json_response['projects'].length).to eq(2) + expect(json_response['shared_projects']).to be_an Array + expect(json_response['shared_projects'].length).to eq(1) + expect(json_response['shared_projects'][0]['id']).to eq(project.id) end it "should not return a non existing group" do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 611dd2a2a88..8a52725a893 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -392,11 +392,47 @@ describe API::API, api: true do before { project } before { project_member } - it 'should return a project by id' do + it 'returns a project by id' do + group = create(:group) + link = create(:project_group_link, project: project, group: group) + get api("/projects/#{project.id}", user) + expect(response).to have_http_status(200) + expect(json_response['id']).to eq(project.id) + expect(json_response['description']).to eq(project.description) + expect(json_response['default_branch']).to eq(project.default_branch) + expect(json_response['tag_list']).to be_an Array + expect(json_response['public']).to be_falsey + expect(json_response['archived']).to be_falsey + expect(json_response['visibility_level']).to be_present + expect(json_response['ssh_url_to_repo']).to be_present + expect(json_response['http_url_to_repo']).to be_present + expect(json_response['web_url']).to be_present + expect(json_response['owner']).to be_a Hash + expect(json_response['owner']).to be_a Hash expect(json_response['name']).to eq(project.name) - expect(json_response['owner']['username']).to eq(user.username) + expect(json_response['path']).to be_present + expect(json_response['issues_enabled']).to be_present + expect(json_response['merge_requests_enabled']).to be_present + expect(json_response['wiki_enabled']).to be_present + expect(json_response['builds_enabled']).to be_present + expect(json_response['snippets_enabled']).to be_present + expect(json_response['container_registry_enabled']).to be_present + expect(json_response['created_at']).to be_present + expect(json_response['last_activity_at']).to be_present + expect(json_response['shared_runners_enabled']).to be_present + expect(json_response['creator_id']).to be_present + expect(json_response['namespace']).to be_present + expect(json_response['avatar_url']).to be_nil + expect(json_response['star_count']).to be_present + expect(json_response['forks_count']).to be_present + expect(json_response['public_builds']).to be_present + expect(json_response['shared_with_groups']).to be_an Array + expect(json_response['shared_with_groups'].length).to eq(1) + expect(json_response['shared_with_groups'][0]['group_id']).to eq(group.id) + expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name) + expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) end it 'should return a project by path name' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 54719cbb8d8..d3dddfb4817 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -293,6 +293,30 @@ describe NotificationService, services: true do end end end + + context "merge request diff note" do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, source_project: project, assignee: user) } + let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) } + + before do + build_team(note.project) + project.team << [merge_request.author, :master] + project.team << [merge_request.assignee, :master] + end + + 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 + + notification.new_note(note) + + expect(SentNotification.last.position).to eq(note.position) + end + end + end end describe 'Issues' do diff --git a/spec/support/capybara_helpers.rb b/spec/support/capybara_helpers.rb index 9b5c3065eed..b57a3493aff 100644 --- a/spec/support/capybara_helpers.rb +++ b/spec/support/capybara_helpers.rb @@ -27,6 +27,14 @@ module CapybaraHelpers end end end + + # Refresh the page. Calling `visit current_url` doesn't seem to work consistently. + # + def refresh + url = current_url + visit 'about:blank' + visit url + end end RSpec.configure do |config| diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 9f9ef20f99b..bb6c84262f6 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -5,19 +5,20 @@ module TestEnv # When developing the seed repository, comment out the branch you will modify. BRANCH_SHA = { - 'empty-branch' => '7efb185', - 'flatten-dir' => 'e56497b', - 'feature' => '0b4bc9a', - 'feature_conflict' => 'bb5206f', - 'fix' => '48f0be4', - 'improve/awesome' => '5937ac0', - 'markdown' => '0ed8c6c', - 'lfs' => 'be93687', - 'master' => '5937ac0', - "'test'" => 'e56497b', - 'orphaned-branch' => '45127a9', - 'binary-encoding' => '7b1cf43', - 'gitattributes' => '5a62481', + 'empty-branch' => '7efb185', + 'flatten-dir' => 'e56497b', + 'feature' => '0b4bc9a', + 'feature_conflict' => 'bb5206f', + 'fix' => '48f0be4', + 'improve/awesome' => '5937ac0', + 'markdown' => '0ed8c6c', + 'lfs' => 'be93687', + 'master' => '5937ac0', + "'test'" => 'e56497b', + 'orphaned-branch' => '45127a9', + 'binary-encoding' => '7b1cf43', + 'gitattributes' => '5a62481', + 'expand-collapse-diffs' => '4842455' } # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily @@ -63,7 +64,7 @@ module TestEnv end def disable_pre_receive - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true) + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) end # Clean /tmp/tests |