Fix test failures, that did occur because of missing previously used `reload_status!` call

app/models/ci/build.rb

@@ -42,24 +42,25 @@ module Ci
       end
       def retry(build, user = nil)
-        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
-        new_build.project = build.project
-        new_build.pipeline = build.pipeline
-        new_build.name = build.name
-        new_build.allow_failure = build.allow_failure
-        new_build.stage = build.stage
-        new_build.stage_idx = build.stage_idx
-        new_build.trigger_request = build.trigger_request
-        new_build.yaml_variables = build.yaml_variables
-        new_build.when = build.when
-        new_build.user = user
-        new_build.environment = build.environment
-        new_build.save
+        new_build = Ci::Build.create(
+          ref: build.ref,
+          tag: build.tag,
+          options: build.options,
+          commands: build.commands,
+          tag_list: build.tag_list,
+          project: build.project,
+          pipeline: build.pipeline,
+          name: build.name,
+          allow_failure: build.allow_failure,
+          stage: build.stage,
+          stage_idx: build.stage_idx,
+          trigger_request: build.trigger_request,
+          yaml_variables: build.yaml_variables,
+          when: build.when,
+          user: user,
+          environment: build.environment,
+          status_event: 'queue'
+        )
         MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build)
         new_build
       end

app/models/commit_status.rb

@@ -62,14 +62,6 @@ class CommitStatus < ActiveRecord::Base
       commit_status.update_attributes finished_at: Time.now
     end
-    after_transition [:created, :pending, :running] => :success do |commit_status|
-      MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status)
-    end
-
-    after_transition any => :failed do |commit_status|
-      MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status)
-    end
-
     # We use around_transition to process pipeline on next stages as soon as possible, before the `after_*` is executed
     around_transition any => [:success, :failed, :canceled] do |commit_status, block|
       block.call
@@ -77,11 +69,17 @@ class CommitStatus < ActiveRecord::Base
       commit_status.pipeline.try(:process!)
     end
-    # Try to update the pipeline status
-
     after_transition do |commit_status, transition|
       commit_status.pipeline.try(:update_status) unless transition.loopback?
     end
+
+    after_transition [:created, :pending, :running] => :success do |commit_status|
+      MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status)
+    end
+
+    after_transition any => :failed do |commit_status|
+      MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status)
+    end
   end
   delegate :sha, :short_sha, to: :pipeline

features/steps/shared/builds.rb

@@ -23,7 +23,7 @@ module SharedBuilds
   end
   step 'project has another build that is running' do
-    create(:ci_build, pipeline: @pipeline, name: 'second build', status: 'running')
+    create(:ci_build, pipeline: @pipeline, name: 'second build', status_event: 'run')
   end
   step 'I visit recent build details page' do

spec/features/pipelines_spec.rb

@@ -12,7 +12,7 @@ describe "Pipelines" do
   end
   describe 'GET /:project/pipelines' do
-    let!(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', status: 'running') }
+    let!(:pipeline) { create(:ci_empty_pipeline, project: project, ref: 'master', status: 'running') }
     [:all, :running, :branches].each do |scope|
       context "displaying #{scope}" do
@@ -31,9 +31,10 @@ describe "Pipelines" do
     end
     context 'cancelable pipeline' do
-      let!(:running) { create(:ci_build, :running, pipeline: pipeline, stage: 'test', commands: 'test') }
+      let!(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') }
       before do
+        build.run
         visit namespace_project_pipelines_path(project.namespace, project)
       end
@@ -49,9 +50,10 @@ describe "Pipelines" do
     end
     context 'retryable pipelines' do
-      let!(:failed) { create(:ci_build, :failed, pipeline: pipeline, stage: 'test', commands: 'test') }
+      let!(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') }
       before do
+        build.drop
         visit namespace_project_pipelines_path(project.namespace, project)
       end
@@ -98,9 +100,10 @@ describe "Pipelines" do
       end
       context 'when failed' do
-        let!(:failed) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') }
+        let!(:status) { create(:generic_commit_status, :pending, pipeline: pipeline, stage: 'test') }
         before do
+          status.drop
           visit namespace_project_pipelines_path(project.namespace, project)
         end

spec/lib/ci/charts_spec.rb

@@ -2,21 +2,23 @@ require 'spec_helper'
 describe Ci::Charts, lib: true do
   context "build_times" do
+    let(:project) { create(:empty_project) }
+    let(:chart) { Ci::Charts::BuildTime.new(project) }
+
+    subject { chart.build_times }
+
     before do
-      @pipeline = FactoryGirl.create(:ci_pipeline)
-      FactoryGirl.create(:ci_build, pipeline: @pipeline)
+      create(:ci_empty_pipeline, project: project, duration: 120)
     end
     it 'returns build times in minutes' do
-      chart = Ci::Charts::BuildTime.new(@pipeline.project)
-      expect(chart.build_times).to eq([2])
+      is_expected.to contain_exactly(2)
     end
     it 'handles nil build times' do
-      create(:ci_pipeline, duration: nil, project: @pipeline.project)
-      chart = Ci::Charts::BuildTime.new(@pipeline.project)
-      expect(chart.build_times).to eq([2, 0])
+      create(:ci_empty_pipeline, project: project, duration: nil)
+      is_expected.to contain_exactly(2, 0)
     end
   end
 end

spec/models/ci/pipeline_spec.rb

@@ -120,22 +120,18 @@ describe Ci::Pipeline, models: true do
     end
   end
-  describe '#update_counters' do
-    let(:pipeline) { create :ci_empty_pipeline, project: project }
-    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.skip
-      end
-      [:status, :started_at, :finished_at, :duration].each do |param|
-        it "#{param}" do
-          expect(pipeline.reload.send(param)).to eq(build.send(param))
-        end
-      end
+  describe '#duration' do
+    let(:current) { Time.now.change(usec: 0) }
+    let!(:build) { create :ci_build, name: 'build1', pipeline: pipeline, started_at: current - 60, finished_at: current }
+    let!(:build2) { create :ci_build, name: 'build2', pipeline: pipeline, started_at: current - 60, finished_at: current }
+    before do
+      build.skip
+      build2.skip
+    end
+    it 'matches sum of builds duration' do
+      expect(pipeline.reload.duration).to eq(build.duration + build2.duration)
     end
   end

spec/requests/api/builds_spec.rb

@@ -9,7 +9,7 @@ describe API::API, api: true do
   let!(:developer) { create(:project_member, :developer, user: user, project: project) }
   let(:reporter) { create(:project_member, :reporter, project: project) }
   let(:guest) { create(:project_member, :guest, project: project) }
-  let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) }
+  let!(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) }
   let!(:build) { create(:ci_build, pipeline: pipeline) }
   describe 'GET /projects/:id/builds ' do
@@ -174,7 +174,11 @@ describe API::API, api: true do
   describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do
     let(:api_user) { reporter.user }
-    let(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
+    let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) }
+
+    before do
+      build.success
+    end
     def path_for_ref(ref = pipeline.ref, job = build.name)
       api("/projects/#{project.id}/builds/artifacts/#{ref}/download?job=#{job}", api_user)