diff options
author | Sean McGivern <sean@gitlab.com> | 2016-09-01 13:59:10 +0100 |
---|---|---|
committer | Alfredo Sumaran <alfredo@gitlab.com> | 2016-10-13 14:16:34 -0500 |
commit | 3f71c43e88c56bb5310c8814cd9f95cafb4f53ef (patch) | |
tree | a2f79497d1aa44c0bbcee6eb5d9f3b18b4f38647 | |
parent | 5c5259335f8bcc4de117c1e36648a269911281fb (diff) | |
download | gitlab-ce-3f71c43e88c56bb5310c8814cd9f95cafb4f53ef.tar.gz |
Allow setting content for resolutions
When reading conflicts:
1. Add a `type` field. `text` works as before, and has `sections`;
`text-editor` is a file with ambiguous conflict markers that can only
be resolved in an editor.
2. Add a `content_path` field pointing to a JSON representation of the
file's content for a single file.
3. Hitting `content_path` returns a similar datastructure to the `file`,
but without the `content_path` and `sections` fields, and with a
`content` field containing the full contents of the file (with
conflict markers).
When writing conflicts:
1. Instead of `sections` being at the top level, they are now in a
`files` array. This matches the read format better.
2. The `files` array contains file hashes, each of which must contain:
a. `new_path`
b. `old_path`
c. EITHER `sections` (which works as before) or `content` (with the
full content of the resolved file).
-rw-r--r-- | app/controllers/application_controller.rb | 7 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 20 | ||||
-rw-r--r-- | app/models/merge_request.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/resolve_service.rb | 24 | ||||
-rw-r--r-- | config/routes/project.rb | 1 | ||||
-rw-r--r-- | lib/gitlab/conflict/file.rb | 51 | ||||
-rw-r--r-- | lib/gitlab/conflict/file_collection.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/conflict/parser.rb | 15 | ||||
-rw-r--r-- | lib/gitlab/conflict/resolution_error.rb | 6 | ||||
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 168 | ||||
-rw-r--r-- | spec/services/merge_requests/resolve_service_spec.rb | 139 |
11 files changed, 395 insertions, 42 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b3455e04c29..bf37421771f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -114,7 +114,12 @@ class ApplicationController < ActionController::Base end def render_404 - render file: Rails.root.join("public", "404"), layout: false, status: "404" + respond_to do |format| + format.json { head :not_found } + format.any do + render file: Rails.root.join("public", "404"), layout: false, status: "404" + end + end end def no_cache_headers diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 869d96b86f4..2c7a0062f2b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -9,15 +9,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :module_enabled before_action :merge_request, only: [ - :edit, :update, :show, :diffs, :commits, :conflicts, :builds, :pipelines, :merge, :merge_check, + :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines, :merge, :merge_check, :ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts, :assign_related_issues ] before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines] - before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :builds, :pipelines] + before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines] before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check] before_action :define_commit_vars, only: [:diffs] before_action :define_diff_comment_vars, only: [:diffs] - before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :pipelines] + before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :conflict_for_path, :pipelines] before_action :close_merge_request_without_source_project, only: [:show, :diffs, :commits, :builds, :pipelines] before_action :apply_diff_view_cookie!, only: [:new_diffs] before_action :build_merge_request, only: [:new, :new_diffs] @@ -33,7 +33,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :authenticate_user!, only: [:assign_related_issues] - before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :resolve_conflicts] + before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :conflict_for_path, :resolve_conflicts] def index @merge_requests = merge_requests_collection @@ -170,6 +170,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + def conflict_for_path + return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui? + + file = @merge_request.conflicts.file_for_path(params[:old_path], params[:new_path]) + + return render_404 unless file + + render json: file, full_content: true + end + def resolve_conflicts return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui? @@ -184,7 +194,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController flash[:notice] = 'All merge conflicts were resolved. The merge request can now be merged.' render json: { redirect_to: namespace_project_merge_request_url(@project.namespace, @project, @merge_request, resolved_conflicts: true) } - rescue Gitlab::Conflict::File::MissingResolution => e + rescue Gitlab::Conflict::ResolutionError => e render status: :bad_request, json: { message: e.message } end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a743bf313ae..1fb0371fe06 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -868,7 +868,7 @@ class MergeRequest < ActiveRecord::Base # files. conflicts.files.each(&:lines) @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0 - rescue Rugged::OdbError, Gitlab::Conflict::Parser::ParserError, Gitlab::Conflict::FileCollection::ConflictSideMissing + rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing @conflicts_can_be_resolved_in_ui = false end end diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb index 19caa038c44..d22a1d3e0ad 100644 --- a/app/services/merge_requests/resolve_service.rb +++ b/app/services/merge_requests/resolve_service.rb @@ -1,5 +1,8 @@ module MergeRequests class ResolveService < MergeRequests::BaseService + class MissingFiles < Gitlab::Conflict::ResolutionError + end + attr_accessor :conflicts, :rugged, :merge_index, :merge_request def execute(merge_request) @@ -10,8 +13,16 @@ module MergeRequests fetch_their_commit! - conflicts.files.each do |file| - write_resolved_file_to_index(file, params[:sections]) + params[:files].each do |file_params| + conflict_file = merge_request.conflicts.file_for_path(file_params[:old_path], file_params[:new_path]) + + write_resolved_file_to_index(conflict_file, file_params) + end + + unless merge_index.conflicts.empty? + missing_files = merge_index.conflicts.map { |file| file[:ours][:path] } + + raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}" end commit_params = { @@ -23,8 +34,13 @@ module MergeRequests project.repository.resolve_conflicts(current_user, merge_request.source_branch, commit_params) end - def write_resolved_file_to_index(file, resolutions) - new_file = file.resolve_lines(resolutions).map(&:text).join("\n") + def write_resolved_file_to_index(file, params) + new_file = if params[:sections] + file.resolve_lines(params[:sections]).map(&:text).join("\n") + elsif params[:content] + file.resolve_content(params[:content]) + end + our_path = file.our_path merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode) diff --git a/config/routes/project.rb b/config/routes/project.rb index f9d58f5d5b2..cbce9cd47a0 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -267,6 +267,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only: get :commits get :diffs get :conflicts + get :conflict_for_path get :builds get :pipelines get :merge_check diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb index dff9e29c6a5..26a9f170298 100644 --- a/lib/gitlab/conflict/file.rb +++ b/lib/gitlab/conflict/file.rb @@ -4,12 +4,12 @@ module Gitlab include Gitlab::Routing.url_helpers include IconsHelper - class MissingResolution < StandardError + class MissingResolution < ResolutionError end CONTEXT_LINES = 3 - attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository + attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository, :type def initialize(merge_file_result, conflict, merge_request:) @merge_file_result = merge_file_result @@ -21,12 +21,24 @@ module Gitlab @match_line_headers = {} end + def content + merge_file_result[:data] + end + # Array of Gitlab::Diff::Line objects def lines - @lines ||= Gitlab::Conflict::Parser.new.parse(merge_file_result[:data], + return @lines if defined?(@lines) + + begin + @type = 'text' + @lines = Gitlab::Conflict::Parser.new.parse(content, our_path: our_path, their_path: their_path, parent_file: self) + rescue Gitlab::Conflict::Parser::ParserError + @type = 'text-editor' + @lines = nil + end end def resolve_lines(resolution) @@ -53,6 +65,14 @@ module Gitlab end.compact end + def resolve_content(resolution) + if resolution == content + raise MissingResolution, "Resolved content has no changes for file #{our_path}" + end + + resolution + end + def highlight_lines! their_file = lines.reject { |line| line.type == 'new' }.map(&:text).join("\n") our_file = lines.reject { |line| line.type == 'old' }.map(&:text).join("\n") @@ -170,21 +190,36 @@ module Gitlab match_line.text = "@@ -#{match_line.old_pos},#{line.old_pos} +#{match_line.new_pos},#{line.new_pos} @@#{header}" end - def as_json(opts = nil) - { + def as_json(opts = {}) + json_hash = { old_path: their_path, new_path: our_path, blob_icon: file_type_icon_class('file', our_mode, our_path), blob_path: namespace_project_blob_path(merge_request.project.namespace, merge_request.project, - ::File.join(merge_request.diff_refs.head_sha, our_path)), - sections: sections + ::File.join(merge_request.diff_refs.head_sha, our_path)) } + + if opts[:full_content] + json_hash.merge(content: content) + else + json_hash.merge!(sections: sections) if type == 'text' + + json_hash.merge(type: type, content_path: content_path) + end + end + + def content_path + conflict_for_path_namespace_project_merge_request_path(merge_request.project.namespace, + merge_request.project, + merge_request, + old_path: their_path, + new_path: our_path) end # Don't try to print merge_request or repository. def inspect - instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode].map do |instance_variable| + instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode, :type].map do |instance_variable| value = instance_variable_get("@#{instance_variable}") "#{instance_variable}=\"#{value}\"" diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index bbd0427a2c8..fa5bd4649d4 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -30,6 +30,10 @@ module Gitlab end end + def file_for_path(old_path, new_path) + files.find { |file| file.their_path == old_path && file.our_path == new_path } + end + def as_json(opts = nil) { target_branch: merge_request.target_branch, diff --git a/lib/gitlab/conflict/parser.rb b/lib/gitlab/conflict/parser.rb index 98e842cded3..ddd657903fb 100644 --- a/lib/gitlab/conflict/parser.rb +++ b/lib/gitlab/conflict/parser.rb @@ -1,19 +1,24 @@ module Gitlab module Conflict class Parser - class ParserError < StandardError + class UnresolvableError < StandardError end - class UnexpectedDelimiter < ParserError + class UnmergeableFile < UnresolvableError end - class MissingEndDelimiter < ParserError + class UnsupportedEncoding < UnresolvableError + end + + # Recoverable errors - the conflict can be resolved in an editor, but not with + # sections. + class ParserError < StandardError end - class UnmergeableFile < ParserError + class UnexpectedDelimiter < ParserError end - class UnsupportedEncoding < ParserError + class MissingEndDelimiter < ParserError end def parse(text, our_path:, their_path:, parent_file: nil) diff --git a/lib/gitlab/conflict/resolution_error.rb b/lib/gitlab/conflict/resolution_error.rb new file mode 100644 index 00000000000..a0f2006bc24 --- /dev/null +++ b/lib/gitlab/conflict/resolution_error.rb @@ -0,0 +1,6 @@ +module Gitlab + module Conflict + class ResolutionError < StandardError + end + end +end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 84298f8bef4..1311b4aa264 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -570,7 +570,7 @@ describe Projects::MergeRequestsController do context 'when the conflicts cannot be resolved in the UI' do before do allow_any_instance_of(Gitlab::Conflict::Parser). - to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnexpectedDelimiter) + to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile) get :conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, @@ -655,6 +655,61 @@ describe Projects::MergeRequestsController do id: merge_request.iid expect(merge_request.reload.title).to eq(merge_request.wipless_title) + end + + describe 'GET conflict_for_path' do + let(:json_response) { JSON.parse(response.body) } + + def conflict_for_path(path) + get :conflict_for_path, + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project.to_param, + id: merge_request_with_conflicts.iid, + old_path: path, + new_path: path, + format: 'json' + end + + context 'when the conflicts cannot be resolved in the UI' do + before do + allow_any_instance_of(Gitlab::Conflict::Parser). + to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile) + + conflict_for_path('files/ruby/regex.rb') + end + + + it 'returns a 404 status code' do + expect(response).to have_http_status(:not_found) + end + end + + context 'when the file does not exist cannot be resolved in the UI' do + before { conflict_for_path('files/ruby/regexp.rb') } + + it 'returns a 404 status code' do + expect(response).to have_http_status(:not_found) + end + end + + context 'with an existing file' do + let(:path) { 'files/ruby/regex.rb' } + + before { conflict_for_path(path) } + + it 'returns a 200 status code' do + expect(response).to have_http_status(:ok) + end + + it 'returns the file in JSON format' do + content = merge_request_with_conflicts.conflicts.file_for_path(path, path).content + + expect(json_response).to include('old_path' => path, + 'new_path' => path, + 'blob_icon' => 'file-text-o', + 'blob_path' => a_string_ending_with(path), + 'content' => content) + end end end @@ -662,22 +717,37 @@ describe Projects::MergeRequestsController do let(:json_response) { JSON.parse(response.body) } let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha } - def resolve_conflicts(sections) + def resolve_conflicts(files) post :resolve_conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, project_id: merge_request_with_conflicts.project.to_param, id: merge_request_with_conflicts.iid, format: 'json', - sections: sections, + files: files, commit_message: 'Commit message' end context 'with valid params' do before do - resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin') + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'sections' => { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) end it 'creates a new commit on the branch' do @@ -692,7 +762,23 @@ describe Projects::MergeRequestsController do context 'when sections are missing' do before do - resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head') + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'sections' => { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' + } + } + ] + + resolve_conflicts(resolved_files) end it 'returns a 400 error' do @@ -700,7 +786,71 @@ describe Projects::MergeRequestsController do end it 'has a message with the name of the first missing section' do - expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9') + expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21') + end + + it 'does not create a new commit' do + expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) + end + end + + context 'when files are missing' do + before do + resolved_files = [ + { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) + end + + it 'returns a 400 error' do + expect(response).to have_http_status(:bad_request) + end + + it 'has a message with the name of the missing file' do + expect(json_response['message']).to include('files/ruby/popen.rb') + end + + it 'does not create a new commit' do + expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) + end + end + + context 'when a file has identical content to the conflict' do + before do + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'content' => merge_request_with_conflicts.conflicts.file_for_path('files/ruby/popen.rb', 'files/ruby/popen.rb').content + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) + end + + it 'returns a 400 error' do + expect(response).to have_http_status(:bad_request) + end + + it 'has a message with the path of the problem file' do + expect(json_response['message']).to include('files/ruby/popen.rb') end it 'does not create a new commit' do diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb index d71932458fa..e667e93bea4 100644 --- a/spec/services/merge_requests/resolve_service_spec.rb +++ b/spec/services/merge_requests/resolve_service_spec.rb @@ -24,15 +24,26 @@ describe MergeRequests::ResolveService do end describe '#execute' do - context 'with valid params' do + context 'with section params' do let(:params) do { - sections: { - '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' - }, + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + sections: { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ], commit_message: 'This is a commit message!' } end @@ -74,8 +85,96 @@ describe MergeRequests::ResolveService do end end - context 'when a resolution is missing' do - let(:invalid_params) { { sections: { '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' } } } + context 'with content and sections params' do + let(:popen_content) { "class Popen\nend" } + + let(:params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: popen_content + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ], + commit_message: 'This is a commit message!' + } + end + + before do + MergeRequests::ResolveService.new(project, user, params).execute(merge_request) + end + + it 'creates a commit with the message' do + expect(merge_request.source_branch_head.message).to eq(params[:commit_message]) + end + + it 'creates a commit with the correct parents' do + expect(merge_request.source_branch_head.parents.map(&:id)). + to eq(['1450cd639e0bc6721eb02800169e464f212cde06', + '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b']) + end + + it 'sets the content to the content given' do + blob = merge_request.source_project.repository.blob_at(merge_request.source_branch_head.sha, + 'files/ruby/popen.rb') + + expect(blob.data).to eq(popen_content) + end + end + + context 'when a resolution section is missing' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' } + } + ], + commit_message: 'This is a commit message!' + } + end + + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } + + it 'raises a MissingResolution error' do + expect { service.execute(merge_request) }. + to raise_error(Gitlab::Conflict::File::MissingResolution) + end + end + + context 'when the content of a file is unchanged' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + content: merge_request.conflicts.file_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb').content + } + ], + commit_message: 'This is a commit message!' + } + end + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } it 'raises a MissingResolution error' do @@ -83,5 +182,27 @@ describe MergeRequests::ResolveService do to raise_error(Gitlab::Conflict::File::MissingResolution) end end + + context 'when a file is missing' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + } + ], + commit_message: 'This is a commit message!' + } + end + + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } + + it 'raises a MissingFiles error' do + expect { service.execute(merge_request) }. + to raise_error(MergeRequests::ResolveService::MissingFiles) + end + end end end |