summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-04-03 16:44:51 +0700
committerShinya Maeda <shinya@gitlab.com>2019-04-05 09:26:56 +0700
commitfa73f4ee196b8c9d28c3b0b035acdd71d71dadb3 (patch)
tree16233cb3f29ad35978af388108a0020b29f49eba
parentb54228ad3d79dc0bd7060128e0b75f68cd1c51d9 (diff)
downloadgitlab-ce-fix-merge-request-relations-with-pipeline-on-mwps.tar.gz
Fix merge requst relationships with pipeline in MWPSServicefix-merge-request-relations-with-pipeline-on-mwps
MWPSService currently uses the old pipeline lookup method. It searches related merge requests with pipeline.ref, however, this doesn't work for attached/detached merge request pipelines.
-rw-r--r--app/services/merge_requests/add_todo_when_build_fails_service.rb4
-rw-r--r--app/services/merge_requests/base_service.rb13
-rw-r--r--changelogs/unreleased/fix-merge-request-relations-with-pipeline-on-mwps.yml5
-rw-r--r--spec/models/ci/build_spec.rb25
-rw-r--r--spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb32
-rw-r--r--spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb15
6 files changed, 77 insertions, 17 deletions
diff --git a/app/services/merge_requests/add_todo_when_build_fails_service.rb b/app/services/merge_requests/add_todo_when_build_fails_service.rb
index 79c43b8e7d5..d3ef892875b 100644
--- a/app/services/merge_requests/add_todo_when_build_fails_service.rb
+++ b/app/services/merge_requests/add_todo_when_build_fails_service.rb
@@ -7,7 +7,7 @@ module MergeRequests
def execute(commit_status)
return if commit_status.allow_failure? || commit_status.retried?
- commit_status_merge_requests(commit_status) do |merge_request|
+ pipeline_merge_requests(commit_status.pipeline) do |merge_request|
todo_service.merge_request_build_failed(merge_request)
end
end
@@ -16,7 +16,7 @@ module MergeRequests
# build is retried
#
def close(commit_status)
- commit_status_merge_requests(commit_status) do |merge_request|
+ pipeline_merge_requests(commit_status.pipeline) do |merge_request|
todo_service.merge_request_build_retried(merge_request)
end
end
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index f968e3693da..8a9e5ebb014 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -99,22 +99,11 @@ module MergeRequests
# rubocop: enable CodeReuse/ActiveRecord
def pipeline_merge_requests(pipeline)
- merge_requests_for(pipeline.ref).each do |merge_request|
+ pipeline.all_merge_requests.opened.each do |merge_request|
next unless pipeline == merge_request.head_pipeline
yield merge_request
end
end
-
- def commit_status_merge_requests(commit_status)
- merge_requests_for(commit_status.ref).each do |merge_request|
- pipeline = merge_request.head_pipeline
-
- next unless pipeline
- next unless pipeline.sha == commit_status.sha
-
- yield merge_request
- end
- end
end
end
diff --git a/changelogs/unreleased/fix-merge-request-relations-with-pipeline-on-mwps.yml b/changelogs/unreleased/fix-merge-request-relations-with-pipeline-on-mwps.yml
new file mode 100644
index 00000000000..9ccc79109d8
--- /dev/null
+++ b/changelogs/unreleased/fix-merge-request-relations-with-pipeline-on-mwps.yml
@@ -0,0 +1,5 @@
+---
+title: Fix MWPS does not work for merge request pipelines
+merge_request: 26906
+author:
+type: fixed
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 697fe3fda06..d0d43ee4462 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -3216,7 +3216,7 @@ describe Ci::Build do
it 'does not try to create a todo' do
project.add_developer(user)
- expect(service).not_to receive(:commit_status_merge_requests)
+ expect(service).not_to receive(:pipeline_merge_requests)
subject.drop!
end
@@ -3252,7 +3252,23 @@ describe Ci::Build do
end
context 'when build is not configured to be retried' do
- subject { create(:ci_build, :running, project: project, user: user) }
+ subject { create(:ci_build, :running, project: project, user: user, pipeline: pipeline) }
+
+ let(:pipeline) do
+ create(:ci_pipeline,
+ project: project,
+ ref: 'feature',
+ sha: merge_request.diff_head_sha,
+ merge_requests_as_head_pipeline: [merge_request])
+ end
+
+ let(:merge_request) do
+ create(:merge_request, :opened,
+ source_branch: 'feature',
+ source_project: project,
+ target_branch: 'master',
+ target_project: project)
+ end
it 'does not retry build' do
expect(described_class).not_to receive(:retry)
@@ -3271,7 +3287,10 @@ describe Ci::Build do
it 'creates a todo' do
project.add_developer(user)
- expect(service).to receive(:commit_status_merge_requests)
+ expect_next_instance_of(TodoService) do |todo_service|
+ expect(todo_service)
+ .to receive(:merge_request_build_failed).with(merge_request)
+ end
subject.drop!
end
diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
index af0a214c00f..39a2ef579dd 100644
--- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
+++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
@@ -77,6 +77,22 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
service.execute(commit_status)
end
end
+
+ context 'when build belongs to a merge request pipeline' do
+ let(:pipeline) do
+ create(:ci_pipeline, source: :merge_request_event,
+ ref: merge_request.merge_ref_path,
+ merge_request: merge_request,
+ merge_requests_as_head_pipeline: [merge_request])
+ end
+
+ let(:commit_status) { create(:ci_build, ref: merge_request.merge_ref_path, pipeline: pipeline) }
+
+ it 'notifies the todo service' do
+ expect(todo_service).to receive(:merge_request_build_failed).with(merge_request)
+ service.execute(commit_status)
+ end
+ end
end
describe '#close' do
@@ -106,6 +122,22 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
service.close(commit_status)
end
end
+
+ context 'when build belongs to a merge request pipeline' do
+ let(:pipeline) do
+ create(:ci_pipeline, source: :merge_request_event,
+ ref: merge_request.merge_ref_path,
+ merge_request: merge_request,
+ merge_requests_as_head_pipeline: [merge_request])
+ end
+
+ let(:commit_status) { create(:ci_build, ref: merge_request.merge_ref_path, pipeline: pipeline) }
+
+ it 'notifies the todo service' do
+ expect(todo_service).to receive(:merge_request_build_retried).with(merge_request)
+ service.close(commit_status)
+ end
+ end
end
describe '#close_all' do
diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
index 52bbd4e794d..1c709df58b6 100644
--- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
+++ b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
@@ -112,6 +112,21 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
service.trigger(unrelated_pipeline)
end
end
+
+ context 'when pipeline is merge request pipeline' do
+ let(:pipeline) do
+ create(:ci_pipeline, :success,
+ source: :merge_request_event,
+ ref: mr_merge_if_green_enabled.merge_ref_path,
+ merge_request: mr_merge_if_green_enabled,
+ merge_requests_as_head_pipeline: [mr_merge_if_green_enabled])
+ end
+
+ it 'merges the associated merge request' do
+ expect(MergeWorker).to receive(:perform_async)
+ service.trigger(pipeline)
+ end
+ end
end
describe "#cancel" do