Fix review commentsrefactor-builds-creation-service

8 files changed, 14 insertions, 17 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 785a8833260..718fe3290c1 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -185,6 +185,7 @@ module Ci
 
     def process!
       Ci::ProcessPipelineService.new(project, user).execute(self)
+      reload_status!
     end
 
     def predefined_variables
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 25b6287b5d6..20713314a25 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -74,10 +74,7 @@ class CommitStatus < ActiveRecord::Base
     around_transition any => [:success, :failed, :canceled] do |commit_status, block|
       block.call
 
-      if commit_status.pipeline
-        commit_status.pipeline.process!
-        commit_status.pipeline.reload_status!
-      end
+      commit_status.pipeline.process! if commit_status.pipeline
     end
   end
 
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index a346ce39ca5..7398fd8e10a 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -46,7 +46,6 @@ module Ci
 
       pipeline.save
       pipeline.process!
-      pipeline.reload_status!
       pipeline
     end
 
diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb
index 1785e9cf4d5..86c4823d18a 100644
--- a/app/services/ci/process_pipeline_service.rb
+++ b/app/services/ci/process_pipeline_service.rb
@@ -26,17 +26,17 @@ module Ci
     end
 
     def process_stage(index)
-      prior_stages_status = status_for_prior_stages(index)
+      current_status = status_for_prior_stages(index)
 
       created_builds_in_stage(index).select do |build|
-        process_build(build, prior_stages_status)
+        process_build(build, current_status)
       end
     end
 
-    def process_build(build, status)
-      return false unless Statuseable::COMPLETED_STATUSES.include?(status)
+    def process_build(build, current_status)
+      return false unless Statuseable::COMPLETED_STATUSES.include?(current_status)
 
-      if valid_statuses_for_when(build.when).include?(status)
+      if valid_statuses_for_when(build.when).include?(current_status)
         build.queue
         true
       else
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index af296fb259b..fdb579ab45c 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -147,6 +147,7 @@ describe Ci::Pipeline, models: true do
 
       it 'executes reload_status! after succeeding dependent object' do
         expect(pipeline).to receive(:reload_status!).and_return(true)
+
         commit_status.success
       end
     end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 8cd25b20650..7ca75d77673 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -89,6 +89,7 @@ describe API::API, api: true  do
 
       it "returns nil for commit without CI" do
         get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
+
         expect(response).to have_http_status(200)
         expect(json_response['status']).to be_nil
       end
@@ -96,14 +97,18 @@ describe API::API, api: true  do
       it "returns status for CI" do
         pipeline = project.ensure_pipeline(project.repository.commit.sha, 'master')
         pipeline.update(status: 'success')
+
         get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
+
         expect(response).to have_http_status(200)
         expect(json_response['status']).to eq(pipeline.status)
       end
 
       it "returns status for CI when pipeline is created" do
         project.ensure_pipeline(project.repository.commit.sha, 'master')
+
         get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
+
         expect(response).to have_http_status(200)
         expect(json_response['status']).to be_nil
       end
diff --git a/spec/services/ci/create_pipeline_builds_service_spec.rb b/spec/services/ci/create_pipeline_builds_service_spec.rb
deleted file mode 100644
index 88057e0cbdb..00000000000
--- a/spec/services/ci/create_pipeline_builds_service_spec.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-require 'spec_helper'
-
-describe Ci::CreatePipelineBuildsService, services: true do
-  let(:project) { create(:ci_project) }
-  let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) }
-  # let(:)
-end
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 89eca2cd768..4aadd009f3e 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -56,6 +56,7 @@ describe Ci::CreatePipelineService, services: true do
                        before: '00000000',
                        after: project.commit.id,
                        commits: [{ message: 'Message' }])
+
       expect(result).not_to be_persisted
       expect(Ci::Pipeline.count).to eq(0)
     end