Update CreateCommitBuildsService to pass tests

4 files changed, 27 insertions, 25 deletions

diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb
index a02b0b8f9b3..f458dee49a6 100644
--- a/app/services/ci/create_builds_service.rb
+++ b/app/services/ci/create_builds_service.rb
@@ -36,7 +36,8 @@ module Ci
                            :stage,
                            :stage_idx)
 
-        build_attrs.merge!(ref: @commit.ref,
+        build_attrs.merge!(commit: @commit,
+                           ref: @commit.ref,
                            tag: @commit.tag,
                            trigger_request: trigger_request,
                            user: user,
diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb
index 3f048157b3f..aa0ec45be8c 100644
--- a/app/services/create_commit_builds_service.rb
+++ b/app/services/create_commit_builds_service.rb
@@ -1,40 +1,41 @@
 class CreateCommitBuildsService
   def execute(project, user, params)
-    return false unless project.builds_enabled?
+    return unless project.builds_enabled?
 
     before_sha = params[:checkout_sha] || params[:before]
     sha = params[:checkout_sha] || params[:after]
     origin_ref = params[:ref]
 
-    unless origin_ref && sha.present?
-      return false
-    end
-
     ref = Gitlab::Git.ref_name(origin_ref)
     tag = Gitlab::Git.tag_ref?(origin_ref)
 
-    # Skip branch removal
-    if sha == Gitlab::Git::BLANK_SHA
-      return false
-    end
-
     commit = Ci::Commit.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
 
-    # Skip creating ci_commit when no gitlab-ci.yml is found
     unless commit.ci_yaml_file
-      return false
+      commit.errors.add(:base, 'No .gitlab-ci.yml file found')
+      return commit
     end
 
+    # Make object as skipped
+    if commit.skip_ci?
+      commit.status = 'skipped'
+      commit.save
+      return commit
+    end
 
-    # Skip creating builds for commits that have [ci skip]
-    unless commit.skip_ci?
-      # Create builds for commit and
-      #   skip saving pipeline when there are no builds
-      return false unless commit.create_builds(user)
+    # Create builds for commit and
+    #   skip saving pipeline when there are no builds
+    unless commit.create_builds(user)
+      # Save object when there are yaml errors
+      unless commit.yaml_errors.present?
+        commit.errors.add(:base, 'No builds created')
+        return commit
+      end
     end
 
     # Create a new ci_commit
     commit.save!
+    commit.touch
     commit
   end
 end
diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb
index 07b875e4f88..d36aca113a1 100644
--- a/spec/models/ci/commit_spec.rb
+++ b/spec/models/ci/commit_spec.rb
@@ -351,8 +351,8 @@ describe Ci::Commit, models: true do
   end
 
   describe '#update_state!' do
-    it 'execute update_state! after touching object' do
-      expect(commit).to receive(:update_state!).and_return(true)
+    it 'execute update_state after touching object' do
+      expect(commit).to receive(:update_state).and_return(true)
       commit.touch
     end
 
@@ -360,7 +360,7 @@ describe Ci::Commit, models: true do
       let(:commit_status) { build :commit_status, commit: commit }
 
       it 'execute update_state after saving dependent object' do
-        expect(commit).to receive(:update_state!).and_return(true)
+        expect(commit).to receive(:update_state).and_return(true)
         commit_status.save
       end
     end
diff --git a/spec/services/create_commit_builds_service_spec.rb b/spec/services/create_commit_builds_service_spec.rb
index b116e3e8fb4..e3202c959d9 100644
--- a/spec/services/create_commit_builds_service_spec.rb
+++ b/spec/services/create_commit_builds_service_spec.rb
@@ -60,7 +60,7 @@ describe CreateCommitBuildsService, services: true do
                                after: '31das312',
                                commits: [{ message: 'Message' }]
                               )
-      expect(result).to be_falsey
+      expect(result).not_to be_persisted
       expect(Ci::Commit.count).to eq(0)
     end
 
@@ -174,7 +174,7 @@ describe CreateCommitBuildsService, services: true do
 
     context 'when there are no jobs for this pipeline' do
       before do
-        config = YAML.dump({ test: { deploy: 'ls', only: ['feature'] } })
+        config = YAML.dump({ test: { script: 'ls', only: ['feature'] } })
         stub_ci_commit_yaml_file(config)
       end
 
@@ -184,9 +184,9 @@ describe CreateCommitBuildsService, services: true do
                                  before: '00000000',
                                  after: '31das312',
                                  commits: [{ message: 'some msg' }])
-
-        expect(result).to be false
+        expect(result).not_to be_persisted
         expect(Ci::Build.all).to be_empty
+        expect(Ci::Commit.count).to eq(0)
       end
     end
   end