From 0367dbf04392a200b5a2e0fcbab6269ff283fa54 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 5 Oct 2015 14:15:15 +0200
Subject: [PATCH] Fix build pipelining

---
 app/models/ci/build.rb        |  5 ++--
 app/models/ci/commit.rb       | 10 +++++++
 spec/models/ci/commit_spec.rb | 55 +++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 89af83d8efc..f35224916ed 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -50,6 +50,7 @@ module Ci
     scope :latest, ->() { where(id: unscope(:select).select('max(id)').group(:name)).order(stage_idx: :asc) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
     scope :for_ref, ->(ref) { where(ref: ref) }
+    scope :similar, ->(build) { where(ref: build.ref, tag: build.tag, trigger_request_id: build.trigger_request_id) }
 
     acts_as_taggable
 
@@ -125,8 +126,8 @@ module Ci
           Ci::WebHookService.new.build_end(build)
         end
 
-        if build.commit.success?
-          build.commit.create_next_builds(build.trigger_request)
+        if build.commit.should_create_next_builds?(build)
+          build.commit.create_next_builds(build.ref, build.tag, build.user, build.trigger_request)
         end
 
         project.execute_services(build)
diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb
index 31da7e8f2b6..c77921979a6 100644
--- a/app/models/ci/commit.rb
+++ b/app/models/ci/commit.rb
@@ -224,6 +224,16 @@ module Ci
       update!(committed_at: DateTime.now)
     end
 
+    def should_create_next_builds?(build)
+      # don't create other builds if this one is retried
+      other_builds = builds.similar(build).latest
+      return false unless other_builds.include?(build)
+
+      other_builds.all? do |build|
+        build.success? || build.ignored?
+      end
+    end
+
     private
 
     def save_yaml_error(error)
diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb
index d18220f119b..91cf96a6666 100644
--- a/spec/models/ci/commit_spec.rb
+++ b/spec/models/ci/commit_spec.rb
@@ -215,4 +215,59 @@ describe Ci::Commit do
       expect(commit.coverage).to be_nil
     end
   end
+
+  describe :should_create_next_builds? do
+    before do
+      @build1 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'master', tag: false, status: :success
+      @build2 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'develop', tag: false, status: :failed
+      @build3 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'master', tag: true, status: :failed
+      @build4 = FactoryGirl.create :ci_build, commit: commit, name: 'build4', ref: 'master', tag: false, status: :success
+    end
+
+    context 'for success' do
+      it 'to create if all succeeded' do
+        expect(commit.should_create_next_builds?(@build4)).to be_truthy
+      end
+    end
+
+    context 'for failed' do
+      before do
+        @build4.update_attributes(status: :failed)
+      end
+
+      it 'to not create' do
+        expect(commit.should_create_next_builds?(@build4)).to be_falsey
+      end
+
+      context 'and ignore failures for current' do
+        before do
+          @build4.update_attributes(allow_failure: true)
+        end
+
+        it 'to create' do
+          expect(commit.should_create_next_builds?(@build4)).to be_truthy
+        end
+      end
+    end
+
+    context 'for running' do
+      before do
+        @build4.update_attributes(status: :running)
+      end
+
+      it 'to not create' do
+        expect(commit.should_create_next_builds?(@build4)).to be_falsey
+      end
+    end
+
+    context 'for retried' do
+      before do
+        @build5 = FactoryGirl.create :ci_build, commit: commit, name: 'build4', ref: 'master', tag: false, status: :failed
+      end
+
+      it 'to not create' do
+        expect(commit.should_create_next_builds?(@build4)).to be_falsey
+      end
+    end
+  end
 end
-- 
GitLab