diff options
author | Sean McGivern <sean@gitlab.com> | 2016-06-28 17:25:32 +0100 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2016-07-08 10:56:55 +0100 |
commit | b6b26692ea44cfeab7e8fd64b7df60852850fce2 (patch) | |
tree | 3f54b14b7b8500d0182d7e5eca653b313eec17e2 /spec | |
parent | 2c650b6f30d2e76cf632d6ce5771859aed67ac48 (diff) | |
download | gitlab-ce-b6b26692ea44cfeab7e8fd64b7df60852850fce2.tar.gz |
Collapse large diffs by default
When rendering a list of diff files, skip those where the diff is over
10 KB and provide an endpoint to render individually instead.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/commit_controller_spec.rb | 246 | ||||
-rw-r--r-- | spec/controllers/projects/commit_controller_spec.rb | 291 | ||||
-rw-r--r-- | spec/controllers/projects/compare_controller_spec.rb | 69 | ||||
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 236 | ||||
-rw-r--r-- | spec/models/merge_request_diff_spec.rb | 47 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 25 |
6 files changed, 595 insertions, 319 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..9679b4f849f 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 - describe 'GET show' 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 "#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 '#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 - 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, 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, path: existing_path) + end + end + + context 'when the path does not exist in the diff' do + before { diff_for_path(id: commit.id, 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, 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, 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..40a068a87bb 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, 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, 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, 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, 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, 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, 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..1b4c4dcd1e5 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -289,101 +289,215 @@ describe Projects::MergeRequestsController do end 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 + describe 'GET #diffs' do + 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, 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, path: existing_path) + end + end + + context 'when the path does not exist in the diff' do + before { diff_for_path(id: merge_request.iid, 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, 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, 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, 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(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(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(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(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(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 - describe 'GET commits' do + describe 'GET #commits' do def go(format: 'html') get :commits, namespace_id: project.namespace.to_param, diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb new file mode 100644 index 00000000000..3f5763b6408 --- /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: ['.gitignore', 'files/ruby/popen.rb', 'files/ruby/string.rb']) } + + it 'only returns diffs that match the paths given' do + expect(diffs.map(&:new_path)).to contain_exactly('.gitignore', '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) } |