From cf292a3f1d3cc17d55131a15d91a53ff31017f5d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 14 Jun 2016 14:09:07 +0200
Subject: [PATCH] Improve code clarity in pipeline create service
---
app/models/ci/pipeline.rb | 2 +-
app/services/create_commit_builds_service.rb | 52 ++++++++++++++-----
.../create_commit_builds_service_spec.rb | 4 +-
3 files changed, 41 insertions(+), 17 deletions(-)
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 83d683b63e4..63639ff2c1f 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -100,7 +100,7 @@ module Ci
def create_builds(user, trigger_request = nil)
build_builds(user, 'success', trigger_request)
- save!
+ save
end
def create_next_builds(build)
diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb
index f2a537c595e..668d0a86549 100644
--- a/app/services/create_commit_builds_service.rb
+++ b/app/services/create_commit_builds_service.rb
@@ -14,26 +14,50 @@ class CreateCommitBuildsService
return false
end
- pipeline = Ci::Pipeline.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
+ @pipeline = Ci::Pipeline.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
- # Skip creating pipeline when no gitlab-ci.yml is found
- unless pipeline.ci_yaml_file
- return pipeline
+ ##
+ # Skip creating pipeline if no gitlab-ci.yml is found
+ #
+ unless @pipeline.ci_yaml_file
+ return false
end
+ ##
# Skip creating builds for commits that have [ci skip]
- if !pipeline.skip_ci? && pipeline.config_processor
- # Create builds for commit
- unless pipeline.build_builds(user)
- pipeline.errors.add(:base, 'No builds created')
- return pipeline
- end
+ # but save pipeline object
+ #
+ if @pipeline.skip_ci?
+ return save_pipeline!
+ end
+
+ ##
+ # Skip creating builds when CI config is invalid
+ # but save pipeline object
+ #
+ unless @pipeline.config_processor
+ return save_pipeline!
+ end
+
+ ##
+ # Skip creating pipeline object if there are no builds for it.
+ #
+ unless @pipeline.build_builds(user)
+ @pipeline.errors.add(:base, 'No builds created')
+ return false
end
- # Create a new pipeline
- pipeline.save!
+ save_pipeline!
+ end
+
+ private
- pipeline.touch
- pipeline
+ ##
+ # Create a new pipeline and touch object to calculate status
+ #
+ def save_pipeline!
+ @pipeline.save!
+ @pipeline.touch
+ @pipeline
end
end
diff --git a/spec/services/create_commit_builds_service_spec.rb b/spec/services/create_commit_builds_service_spec.rb
index 08cbc9beb5c..50ce9659c10 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).not_to be_persisted
+ expect(result).to be_falsey
expect(Ci::Pipeline.count).to eq(0)
end
@@ -184,7 +184,7 @@ describe CreateCommitBuildsService, services: true do
before: '00000000',
after: '31das312',
commits: [{ message: 'some msg' }])
- expect(result).not_to be_persisted
+ expect(result).to be_falsey
expect(Ci::Build.all).to be_empty
expect(Ci::Pipeline.count).to eq(0)
end
--
GitLab