Merge branch 'refactor-builds-creation-service' into pipeline-hooks-without-touchpipeline-hooks-without-touch

7 files changed, 56 insertions, 12 deletions

diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb
index 1e5c479616c..995f2080f10 100644
--- a/spec/factories/commit_statuses.rb
+++ b/spec/factories/commit_statuses.rb
@@ -7,6 +7,30 @@ FactoryGirl.define do
     started_at 'Tue, 26 Jan 2016 08:21:42 +0100'
     finished_at 'Tue, 26 Jan 2016 08:23:42 +0100'
 
+    trait :success do
+      status 'success'
+    end
+
+    trait :failed do
+      status 'failed'
+    end
+
+    trait :canceled do
+      status 'canceled'
+    end
+
+    trait :running do
+      status 'running'
+    end
+
+    trait :pending do
+      status 'pending'
+    end
+
+    trait :created do
+      status 'created'
+    end
+
     after(:build) do |build, evaluator|
       build.project = build.pipeline.project
     end
diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb
index 1256f73196b..f88b8f8e60b 100644
--- a/spec/features/pipelines_spec.rb
+++ b/spec/features/pipelines_spec.rb
@@ -33,7 +33,10 @@ describe "Pipelines" do
     context 'cancelable pipeline' do
       let!(:running) { create(:ci_build, :running, pipeline: pipeline, stage: 'test', commands: 'test') }
 
-      before { visit namespace_project_pipelines_path(project.namespace, project) }
+      before do
+        pipeline.reload_status!
+        visit namespace_project_pipelines_path(project.namespace, project)
+      end
 
       it { expect(page).to have_link('Cancel') }
       it { expect(page).to have_selector('.ci-running') }
@@ -49,7 +52,10 @@ describe "Pipelines" do
     context 'retryable pipelines' do
       let!(:failed) { create(:ci_build, :failed, pipeline: pipeline, stage: 'test', commands: 'test') }
 
-      before { visit namespace_project_pipelines_path(project.namespace, project) }
+      before do
+        pipeline.reload_status!
+        visit namespace_project_pipelines_path(project.namespace, project)
+      end
 
       it { expect(page).to have_link('Retry') }
       it { expect(page).to have_selector('.ci-failed') }
@@ -80,7 +86,10 @@ describe "Pipelines" do
       context 'when running' do
         let!(:running) { create(:generic_commit_status, status: 'running', pipeline: pipeline, stage: 'test') }
 
-        before { visit namespace_project_pipelines_path(project.namespace, project) }
+        before do
+          pipeline.reload_status!
+          visit namespace_project_pipelines_path(project.namespace, project)
+        end
 
         it 'is not cancelable' do
           expect(page).not_to have_link('Cancel')
@@ -92,9 +101,12 @@ describe "Pipelines" do
       end
 
       context 'when failed' do
-        let!(:running) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') }
+        let!(:failed) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') }
 
-        before { visit namespace_project_pipelines_path(project.namespace, project) }
+        before do
+          pipeline.reload_status!
+          visit namespace_project_pipelines_path(project.namespace, project)
+        end
 
         it 'is not retryable' do
           expect(page).not_to have_link('Retry')
diff --git a/spec/lib/ci/charts_spec.rb b/spec/lib/ci/charts_spec.rb
index 034ea098193..2cd6b00dad6 100644
--- a/spec/lib/ci/charts_spec.rb
+++ b/spec/lib/ci/charts_spec.rb
@@ -5,6 +5,7 @@ describe Ci::Charts, lib: true do
     before do
       @pipeline = FactoryGirl.create(:ci_pipeline)
       FactoryGirl.create(:ci_build, pipeline: @pipeline)
+      @pipeline.reload_status!
     end
 
     it 'returns build times in minutes' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 79141a9d94e..0a57734ca6e 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -143,7 +143,7 @@ describe Ci::Pipeline, models: true do
     let(:pipeline) { create :ci_empty_pipeline, project: project }
 
     context 'dependent objects' do
-      let(:commit_status) { create :commit_status, pipeline: pipeline }
+      let(:commit_status) { create :commit_status, :pending, pipeline: pipeline }
 
       it 'executes reload_status! after succeeding dependent object' do
         expect(pipeline).to receive(:reload_status!).and_return(true)
@@ -151,16 +151,17 @@ describe Ci::Pipeline, models: true do
       end
     end
 
-    context 'reload state' do
+    context 'updates' do
       let(:current) { Time.now.change(usec: 0) }
       let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 }
 
       before do
-        build.success
+        build
+        pipeline.reload_status!
       end
 
       [:status, :started_at, :finished_at, :duration].each do |param|
-        it "update #{param}" do
+        it "#{param}" do
           expect(pipeline.send(param)).to eq(build.send(param))
         end
       end
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index 966d302dfd3..a4cdd8f3140 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -238,6 +238,10 @@ describe API::API, api: true do
         it { expect(response.headers).to include(download_headers) }
       end
 
+      before do
+        pipeline.reload_status!
+      end
+
       context 'with regular branch' do
         before do
           pipeline.update(ref: 'master',
diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb
index 3a3e3efe709..259062406c7 100644
--- a/spec/services/ci/image_for_build_service_spec.rb
+++ b/spec/services/ci/image_for_build_service_spec.rb
@@ -5,8 +5,8 @@ module Ci
     let(:service) { ImageForBuildService.new }
     let(:project) { FactoryGirl.create(:empty_project) }
     let(:commit_sha) { '01234567890123456789' }
-    let(:commit) { project.ensure_pipeline(commit_sha, 'master') }
-    let(:build) { FactoryGirl.create(:ci_build, pipeline: commit) }
+    let(:pipeline) { project.ensure_pipeline(commit_sha, 'master') }
+    let(:build) { FactoryGirl.create(:ci_build, pipeline: pipeline) }
 
     describe '#execute' do
       before { build }
@@ -14,6 +14,7 @@ module Ci
       context 'branch name' do
         before { allow(project).to receive(:commit).and_return(OpenStruct.new(sha: commit_sha)) }
         before { build.run! }
+        before { pipeline.reload_status! }
         let(:image) { service.execute(project, ref: 'master') }
 
         it { expect(image).to be_kind_of(OpenStruct) }
@@ -31,6 +32,7 @@ module Ci
 
       context 'commit sha' do
         before { build.run! }
+        before { pipeline.reload_status! }
         let(:image) { service.execute(project, sha: build.sha) }
 
         it { expect(image).to be_kind_of(OpenStruct) }
diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb
index e094a6077a1..520e906b21f 100644
--- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb
+++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb
@@ -110,9 +110,9 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
 
     context 'properly handles multiple stages' do
       let(:ref) { mr_merge_if_green_enabled.source_branch }
-      let(:pipeline) { create(:ci_pipeline_without_jobs, ref: mr_merge_if_green_enabled.source_branch, project: project) }
       let!(:build) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'build', stage: 'build') }
       let!(:test) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'test', stage: 'test') }
+      let(:pipeline) { create(:ci_empty_pipeline, ref: mr_merge_if_green_enabled.source_branch, project: project) }
 
       before do
         # This behavior of MergeRequest: we instantiate a new object