Make commit_spec run

app/models/ci/build.rb

@@ -80,6 +80,8 @@ module Ci
 
       def retry(build)
         new_build = Ci::Build.new(status: :pending)
+        new_build.ref = build.ref
+        new_build.tag = build.tag
         new_build.options = build.options
         new_build.commands = build.commands
         new_build.tag_list = build.tag_list
@@ -141,7 +143,7 @@ module Ci
       state :canceled, value: 'canceled'
     end
 
-    delegate :sha, :short_sha, :before_sha, :ref, :project,
+    delegate :sha, :short_sha, :project,
       to: :commit, prefix: false
 
     def trace_html

app/models/ci/commit.rb

@@ -58,14 +58,6 @@ module Ci
       end
     end
 
-    def new_branch?
-      before_sha == Ci::Git::BLANK_SHA
-    end
-
-    def compare?
-      !new_branch?
-    end
-
     def git_author_name
       commit_data.author_name if commit_data
     end
@@ -78,10 +70,6 @@ module Ci
       commit_data.message if commit_data
     end
 
-    def short_before_sha
-      Ci::Commit.truncate_sha(before_sha)
-    end
-
     def short_sha
       Ci::Commit.truncate_sha(sha)
     end
@@ -99,7 +87,22 @@ module Ci
     def create_builds(ref, tag, user, trigger_request = nil)
       return if skip_ci? && trigger_request.blank?
       return unless config_processor
-      CreateBuildsService.new.execute(self, config_processor, ref, tag, user, trigger_request)
+      config_processor.stages.any? do |stage|
+        CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request).present?
+      end
+    end
+
+    def create_next_builds(ref, tag, user, trigger_request)
+      return if skip_ci? && trigger_request.blank?
+      return unless config_processor
+
+      stages = builds.where(ref: ref, tag: tag, trigger_request: trigger_request).group_by(&:stage)
+
+      config_processor.stages.any? do |stage|
+        unless stages.include?(stage)
+          CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request).present?
+        end
+      end
     end
 
     def refs
@@ -111,7 +114,14 @@ module Ci
     end
 
     def builds_without_retry
-      builds.latest
+      @builds_without_retry ||=
+        begin
+          grouped_builds = builds.group_by(&:name)
+          latest_builds = grouped_builds.map do |name, builds|
+            builds.sort_by(&:id).last
+          end
+          latest_builds.sort_by(&:stage_idx)
+        end
     end
 
     def retried_builds
@@ -125,32 +135,35 @@ module Ci
         return 'failed'
       elsif builds.none?
         return 'skipped'
-      end
-
-      statuses = builds_without_retry.ignore_failures.pluck(:status)
-      if statuses.all? { |status| status == 'success' }
-        return 'success'
-      elsif statuses.all? { |status| status == 'pending' }
-        return 'pending'
-      elsif statuses.include?('running') || statuses.include?('pending')
-        return 'running'
-      elsif statuses.all? { |status| status == 'canceled' }
-        return 'canceled'
+      elsif success?
+        'success'
+      elsif pending?
+        'pending'
+      elsif running?
+        'running'
+      elsif canceled?
+        'canceled'
       else
-        return 'failed'
+        'failed'
       end
     end
 
     def pending?
-      status == 'pending'
+      builds_without_retry.all? do |build|
+        build.pending?
+      end
     end
 
     def running?
-      status == 'running'
+      builds_without_retry.any? do |build|
+        build.running? || build.pending?
+      end
     end
 
     def success?
-      status == 'success'
+      builds_without_retry.all? do |build|
+        build.success? || build.ignored?
+      end
     end
 
     def failed?
@@ -158,7 +171,9 @@ module Ci
     end
 
     def canceled?
-      status == 'canceled'
+      builds_without_retry.all? do |build|
+        build.canceled?
+      end
     end
 
     def duration

app/services/ci/create_builds_service.rb

 module Ci
   class CreateBuildsService
-    def execute(commit, ref, tag, user, config_processor, trigger_request)
-      config_processor.stages.any? do |stage|
-        builds_attrs = config_processor.builds_for_stage_and_ref(stage, ref, tag)
-        builds_attrs.map do |build_attrs|
-          # don't create the same build twice
-          unless commit.builds.find_by_name_and_trigger_request(name: build_attrs[:name], ref: ref, tag: tag, trigger_request: trigger_request)
-            commit.builds.create!({
-                             name: build_attrs[:name],
-                             commands: build_attrs[:script],
-                             tag_list: build_attrs[:tags],
-                             options: build_attrs[:options],
-                             allow_failure: build_attrs[:allow_failure],
-                             stage: build_attrs[:stage],
-                             stage_idx: build_attrs[:stage_idx],
-                             trigger_request: trigger_request,
-                             ref: ref,
-                             tag: tag,
-                             user: user,
-                           })
-          end
-        end
+    def execute(commit, stage, ref, tag, user, trigger_request)
+      builds_attrs = commit.config_processor.builds_for_stage_and_ref(stage, ref, tag)
+
+      builds_attrs.map do |build_attrs|
+        build_attrs.slice!(:name,
+                           :commands,
+                           :tag_list,
+                           :options,
+                           :allow_failure,
+                           :stage,
+                           :stage_idx)
+
+        build_attrs.merge!(ref: ref,
+                           tag: tag,
+                           trigger_request: trigger_request,
+                           user: user)
+
+        commit.builds.create!(build_attrs)
       end
     end
   end

app/services/ci/web_hook_service.rb

@@ -28,8 +28,6 @@ module Ci
         gitlab_url: project.gitlab_url,
         ref: build.ref,
         sha: build.sha,
-        before_sha: build.before_sha,
-        push_data: build.commit.push_data
       })
     end
   end

app/views/ci/builds/show.html.haml

@@ -122,11 +122,6 @@
         Commit
         .pull-right
           %small #{build_commit_link @build}
-
-      - if @build.commit.compare?
-        %p
-          %span.attr-name Compare:
-          #{build_compare_link @build}
       %p
         %span.attr-name Branch:
         #{build_ref_link @build}

app/views/ci/commits/show.html.haml

@@ -9,14 +9,9 @@
   .gray-content-block.second-block
     .row
       .col-sm-6
-        - if @commit.compare?
-          %p
-            %span.attr-name Compare:
-            #{gitlab_compare_link(@project, @commit.short_before_sha, @commit.short_sha)}
-        - else
-          %p
-            %span.attr-name Commit:
-            #{gitlab_commit_link(@project, @commit.sha)}
+       %p
+          %span.attr-name Commit:
+          #{gitlab_commit_link(@project, @commit.sha)}
       .col-sm-6
         - if @commit.git_author_name || @commit.git_author_email
           %p

lib/ci/gitlab_ci_yaml_processor.rb

@@ -87,8 +87,8 @@ module Ci
       {
         stage_idx: stages.index(job[:stage]),
         stage: job[:stage],
-        script: "#{@before_script.join("\n")}\n#{normalize_script(job[:script])}",
-        tags: job[:tags] || [],
+        commands: "#{@before_script.join("\n")}\n#{normalize_script(job[:script])}",
+        tag_list: job[:tags] || [],
         name: name,
         only: job[:only],
         except: job[:except],

spec/factories/ci/commits.rb

@@ -23,20 +23,26 @@ FactoryGirl.define do
     gl_project factory: :empty_project
 
     factory :ci_commit_without_jobs do
-      after(:create) do |commit, evaluator|
+      after(:build) do |commit|
         allow(commit).to receive(:ci_yaml_file) { YAML.dump({}) }
       end
     end
 
     factory :ci_commit_with_one_job do
-      after(:create) do |commit, evaluator|
-        allow(commit).to receive(:ci_yaml_file) { YAML.dump({ rspec: { script: "ls" } }) }
+      after(:build) do |commit|
+        allow(commit).to receive(:ci_yaml_file) { YAML.dump({rspec: {script: "ls"}}) }
       end
     end
 
     factory :ci_commit_with_two_jobs do
-      after(:create) do |commit, evaluator|
-        allow(commit).to receive(:ci_yaml_file) { YAML.dump({ rspec: { script: "ls" }, spinach: { script: "ls" } }) }
+      after(:build) do |commit|
+        allow(commit).to receive(:ci_yaml_file) { YAML.dump({rspec: {script: "ls"}, spinach: {script: "ls"}}) }
+      end
+    end
+
+    factory :ci_commit_yaml_stub do
+      after(:build) do |commit|
+        allow(commit).to receive(:ci_yaml_file) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) }
       end
     end
   end

spec/models/ci/build_spec.rb

@@ -28,11 +28,14 @@ require 'spec_helper'
 describe Ci::Build do
   let(:project) { FactoryGirl.create :ci_project }
   let(:gl_project) { FactoryGirl.create :empty_project, gitlab_ci_project: project }
-  let(:commit) { FactoryGirl.create :ci_commit, gl_project: gl_project }
+  let(:commit) { FactoryGirl.create :ci_commit_yaml_stub, gl_project: gl_project }
   let(:build) { FactoryGirl.create :ci_build, commit: commit }
+  subject { build }
 
   it { is_expected.to belong_to(:commit) }
+  it { is_expected.to belong_to(:user) }
   it { is_expected.to validate_presence_of :status }
+  it { is_expected.to validate_presence_of :ref }
 
   it { is_expected.to respond_to :success? }
   it { is_expected.to respond_to :failed? }
@@ -236,12 +239,6 @@ describe Ci::Build do
     it { is_expected.to eq(options) }
   end
 
-  describe :ref do
-    subject { build.ref }
-
-    it { is_expected.to eq(commit.ref) }
-  end
-
   describe :sha do
     subject { build.sha }
 
@@ -254,12 +251,6 @@ describe Ci::Build do
     it { is_expected.to eq(commit.short_sha) }
   end
 
-  describe :before_sha do
-    subject { build.before_sha }
-
-    it { is_expected.to eq(commit.before_sha) }
-  end
-
   describe :allow_git_fetch do
     subject { build.allow_git_fetch }
 
@@ -359,4 +350,38 @@ describe Ci::Build do
       end
     end
   end
+
+  describe :project_recipients do
+    let (:pusher_email) { 'pusher@gitlab.test' }
+    let (:user) { User.new(notification_email: pusher_email) }
+    subject { build.project_recipients }
+
+    before do
+      build.update_attributes(user: user)
+    end
+
+    it 'should return pusher_email as only recipient when no additional recipients are given' do
+      project.update_attributes(email_add_pusher: true,
+                                email_recipients: '')
+      is_expected.to eq([pusher_email])
+    end
+
+    it 'should return pusher_email and additional recipients' do
+      project.update_attributes(email_add_pusher: true,
+                                email_recipients: 'rec1 rec2')
+      is_expected.to eq(['rec1', 'rec2', pusher_email])
+    end
+
+    it 'should return recipients' do
+      project.update_attributes(email_add_pusher: false,
+                                email_recipients: 'rec1 rec2')
+      is_expected.to eq(['rec1', 'rec2'])
+    end
+
+    it 'should return unique recipients only' do
+      project.update_attributes(email_add_pusher: true,
+                                email_recipients: "rec1 rec1 #{pusher_email}")
+      is_expected.to eq(['rec1', pusher_email])
+    end
+  end
 end

spec/models/ci/commit_spec.rb

@@ -21,15 +21,10 @@ describe Ci::Commit do
   let(:project) { FactoryGirl.create :ci_project }
   let(:gl_project) { FactoryGirl.create :empty_project, gitlab_ci_project: project }
   let(:commit) { FactoryGirl.create :ci_commit, gl_project: gl_project }
-  let(:commit_with_project) { FactoryGirl.create :ci_commit, gl_project: gl_project }
-  let(:config_processor) { Ci::GitlabCiYamlProcessor.new(gitlab_ci_yaml) }
 
   it { is_expected.to belong_to(:gl_project) }
   it { is_expected.to have_many(:builds) }
-  it { is_expected.to validate_presence_of :before_sha }
   it { is_expected.to validate_presence_of :sha }
-  it { is_expected.to validate_presence_of :ref }
-  it { is_expected.to validate_presence_of :push_data }
 
   it { is_expected.to respond_to :git_author_name }
   it { is_expected.to respond_to :git_author_email }
@@ -59,53 +54,6 @@ describe Ci::Commit do
     end
   end
 
-  describe :project_recipients do
-
-    context 'always sending notification' do
-      it 'should return commit_pusher_email as only recipient when no additional recipients are given' do
-        project = FactoryGirl.create :ci_project,
-          email_add_pusher: true,
-          email_recipients: ''
-        gl_project = FactoryGirl.create :empty_project, gitlab_ci_project: project
-        commit =  FactoryGirl.create :ci_commit, gl_project: gl_project
-        expected = 'commit_pusher_email'
-        allow(commit).to receive(:push_data) { { user_email: expected } }
-        expect(commit.project_recipients).to eq([expected])
-      end
-
-      it 'should return commit_pusher_email and additional recipients' do
-        project = FactoryGirl.create :ci_project,
-          email_add_pusher: true,
-          email_recipients: 'rec1 rec2'
-        gl_project = FactoryGirl.create :empty_project, gitlab_ci_project: project
-        commit = FactoryGirl.create :ci_commit, gl_project: gl_project
-        expected = 'commit_pusher_email'
-        allow(commit).to receive(:push_data) { { user_email: expected } }
-        expect(commit.project_recipients).to eq(['rec1', 'rec2', expected])
-      end
-
-      it 'should return recipients' do
-        project = FactoryGirl.create :ci_project,
-          email_add_pusher: false,
-          email_recipients: 'rec1 rec2'
-        gl_project = FactoryGirl.create :empty_project, gitlab_ci_project: project
-        commit = FactoryGirl.create :ci_commit, gl_project: gl_project
-        expect(commit.project_recipients).to eq(['rec1', 'rec2'])
-      end
-
-      it 'should return unique recipients only' do
-        project = FactoryGirl.create :ci_project,
-          email_add_pusher: true,
-          email_recipients: 'rec1 rec1 rec2'
-        gl_project = FactoryGirl.create :empty_project, gitlab_ci_project: project
-        commit = FactoryGirl.create :ci_commit, gl_project: gl_project
-        expected = 'rec2'
-        allow(commit).to receive(:push_data) { { user_email: expected } }
-        expect(commit.project_recipients).to eq(['rec1', 'rec2'])
-      end
-    end
-  end
-
   describe :valid_commit_sha do
     context 'commit.sha can not start with 00000000' do
       before do
@@ -117,14 +65,6 @@ describe Ci::Commit do
     end
   end
 
-  describe :compare? do
-    subject { commit_with_project.compare? }
-
-    context 'if commit.before_sha are not nil' do
-      it { is_expected.to be_truthy }
-    end
-  end
-
   describe :short_sha do
     subject { commit.short_before_sha }
 
@@ -144,36 +84,51 @@ describe Ci::Commit do
   end
 
   describe :create_next_builds do
-    before do
-      allow(commit).to receive(:config_processor).and_return(config_processor)
+  end
+
+  describe :create_builds do
+    let(:commit) { FactoryGirl.create :ci_commit_yaml_stub, gl_project: gl_project }
+
+    def create_builds(trigger_request = nil)
+      commit.create_builds('master', false, nil, trigger_request)
     end
 
-    it "creates builds for next type" do
-      expect(commit.create_builds).to be_truthy
+    def create_next_builds(trigger_request = nil)
+      commit.create_next_builds('master', false, nil, trigger_request)
+    end
+
+    it 'creates builds' do
+      expect(create_builds).to be_truthy
       commit.builds.reload
       expect(commit.builds.size).to eq(2)
 
-      expect(commit.create_next_builds(nil)).to be_truthy
+      expect(create_next_builds).to be_truthy
       commit.builds.reload
       expect(commit.builds.size).to eq(4)
 
-      expect(commit.create_next_builds(nil)).to be_truthy
+      expect(create_next_builds).to be_truthy
       commit.builds.reload
       expect(commit.builds.size).to eq(5)
 
-      expect(commit.create_next_builds(nil)).to be_falsey
+      expect(create_next_builds).to be_falsey
     end
-  end
 
-  describe :create_builds do
-    before do
-      allow(commit).to receive(:config_processor).and_return(config_processor)
-    end
+    context 'for different ref' do
+      def create_develop_builds
+        commit.create_builds('develop', false, nil, nil)
+      end
 
-    it 'creates builds' do
-      expect(commit.create_builds).to be_truthy
-      commit.builds.reload
-      expect(commit.builds.size).to eq(2)
+      it 'creates builds' do
+        expect(create_builds).to be_truthy
+        commit.builds.reload
+        expect(commit.builds.size).to eq(2)
+
+        expect(create_develop_builds).to be_truthy
+        commit.builds.reload
+        expect(commit.builds.size).to eq(4)
+        expect(commit.refs.size).to eq(2)
+        expect(commit.builds.pluck(:name).uniq.size).to eq(2)
+      end
     end
 
     context 'for build triggers' do
@@ -181,40 +136,39 @@ describe Ci::Commit do
       let(:trigger_request) { FactoryGirl.create :ci_trigger_request, commit: commit, trigger: trigger }
 
       it 'creates builds' do
-        expect(commit.create_builds(trigger_request)).to be_truthy
+        expect(create_builds(trigger_request)).to be_truthy
         commit.builds.reload
         expect(commit.builds.size).to eq(2)
       end
 
       it 'rebuilds commit' do
-        expect(commit.create_builds).to be_truthy
+        expect(create_builds).to be_truthy
         commit.builds.reload
         expect(commit.builds.size).to eq(2)
 
-        expect(commit.create_builds(trigger_request)).to be_truthy
+        expect(create_builds(trigger_request)).to be_truthy
         commit.builds.reload
         expect(commit.builds.size).to eq(4)
       end
 
       it 'creates next builds' do
-        expect(commit.create_builds(trigger_request)).to be_truthy
+        expect(create_builds(trigger_request)).to be_truthy
         commit.builds.reload
         expect(commit.builds.size).to eq(2)
 
-        expect(commit.create_next_builds(trigger_request)).to be_truthy
+        expect(create_next_builds(trigger_request)).to be_truthy
         commit.builds.reload
         expect(commit.builds.size).to eq(4)
       end
 
       context 'for [ci skip]' do
         before do
-          commit.push_data[:commits][0][:message] = 'skip this commit [ci skip]'
-          commit.save
+          allow(commit).to receive(:git_commit_message) { 'message [ci skip]' }
         end
 
         it 'rebuilds commit' do
           expect(commit.status).to eq('skipped')
-          expect(commit.create_builds(trigger_request)).to be_truthy
+          expect(create_builds(trigger_request)).to be_truthy
           commit.builds.reload
           expect(commit.builds.size).to eq(2)
           expect(commit.status).to eq('pending')

spec/spec_helper.rb

@@ -42,4 +42,8 @@ RSpec.configure do |config|
   end
 end
 
+FactoryGirl::SyntaxRunner.class_eval do
+  include RSpec::Mocks::ExampleMethods
+end
+
 ActiveRecord::Migration.maintain_test_schema!