Commit 56449cc6 authored by Kamil Trzcinski's avatar Kamil Trzcinski

Fix Merge When Succeeded for multiple stages

Use around_transition to trigger build creation for next stages
parent 2cc9a42c
...@@ -52,6 +52,7 @@ v 8.5.0 (unreleased) ...@@ -52,6 +52,7 @@ v 8.5.0 (unreleased)
- Improve UI consistency between projects and groups lists - Improve UI consistency between projects and groups lists
- Add sort dropdown to dashboard projects page - Add sort dropdown to dashboard projects page
- Fixed logo animation on Safari (Roman Rott) - Fixed logo animation on Safari (Roman Rott)
- Fix Merge When Succeeded when multiple stages
- Hide remove source branch button when the MR is merged but new commits are pushed (Zeger-Jan van de Weg) - Hide remove source branch button when the MR is merged but new commits are pushed (Zeger-Jan van de Weg)
- In seach autocomplete show only groups and projects you are member of - In seach autocomplete show only groups and projects you are member of
- Don't process cross-reference notes from forks - Don't process cross-reference notes from forks
......
...@@ -107,15 +107,18 @@ module Ci ...@@ -107,15 +107,18 @@ module Ci
end end
state_machine :status, initial: :pending do state_machine :status, initial: :pending do
after_transition pending: :running do |build, transition| after_transition pending: :running do |build|
build.execute_hooks build.execute_hooks
end end
after_transition any => [:success, :failed, :canceled] do |build, transition| # We use around_transition to create builds for next stage as soon as possible, before the `after_*` is executed
return unless build.project around_transition any => [:success, :failed, :canceled] do |build, block|
block.call
build.commit.create_next_builds(build) if build.commit
end
after_transition any => [:success, :failed, :canceled] do |build|
build.update_coverage build.update_coverage
build.commit.create_next_builds(build)
build.execute_hooks build.execute_hooks
end end
end end
...@@ -179,6 +182,7 @@ module Ci ...@@ -179,6 +182,7 @@ module Ci
end end
def update_coverage def update_coverage
return unless project
coverage_regex = project.build_coverage_regex coverage_regex = project.build_coverage_regex
return unless coverage_regex return unless coverage_regex
coverage = extract_coverage(trace, coverage_regex) coverage = extract_coverage(trace, coverage_regex)
...@@ -334,6 +338,7 @@ module Ci ...@@ -334,6 +338,7 @@ module Ci
end end
def execute_hooks def execute_hooks
return unless project
build_data = Gitlab::BuildDataBuilder.build(self) build_data = Gitlab::BuildDataBuilder.build(self)
project.execute_hooks(build_data.dup, :build_hooks) project.execute_hooks(build_data.dup, :build_hooks)
project.execute_services(build_data.dup, :build_hooks) project.execute_services(build_data.dup, :build_hooks)
......
...@@ -75,16 +75,16 @@ class CommitStatus < ActiveRecord::Base ...@@ -75,16 +75,16 @@ class CommitStatus < ActiveRecord::Base
transition [:pending, :running] => :canceled transition [:pending, :running] => :canceled
end end
after_transition pending: :running do |build, transition| after_transition pending: :running do |commit_status|
build.update_attributes started_at: Time.now commit_status.update_attributes started_at: Time.now
end end
after_transition any => [:success, :failed, :canceled] do |build, transition| after_transition any => [:success, :failed, :canceled] do |commit_status|
build.update_attributes finished_at: Time.now commit_status.update_attributes finished_at: Time.now
end end
after_transition [:pending, :running] => :success do |build, transition| after_transition [:pending, :running] => :success do |commit_status|
MergeRequests::MergeWhenBuildSucceedsService.new(build.commit.project, nil).trigger(build) MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.commit.project, nil).trigger(commit_status)
end end
state :pending, value: 'pending' state :pending, value: 'pending'
......
...@@ -63,6 +63,36 @@ describe MergeRequests::MergeWhenBuildSucceedsService do ...@@ -63,6 +63,36 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
expect(MergeWorker).to receive(:perform_async) expect(MergeWorker).to receive(:perform_async)
service.trigger(build) service.trigger(build)
end end
context 'properly handles multiple stages' do
let(:ref) { mr_merge_if_green_enabled.source_branch }
let(:build) { create(:ci_build, commit: ci_commit, ref: ref, name: 'build', stage: 'build') }
let(:test) { create(:ci_build, commit: ci_commit, ref: ref, name: 'test', stage: 'test') }
before do
# This behavior of MergeRequest: we instantiate a new object
allow_any_instance_of(MergeRequest).to receive(:ci_commit).and_wrap_original do
Ci::Commit.find(ci_commit.id)
end
# We create test after the build
allow(ci_commit).to receive(:create_next_builds).and_wrap_original do
test
end
end
it "doesn't merge if some stages failed" do
expect(MergeWorker).to_not receive(:perform_async)
build.success
test.drop
end
it 'merge when all stages succeeded' do
expect(MergeWorker).to receive(:perform_async)
build.success
test.success
end
end
end end
describe "#cancel" do describe "#cancel" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment