diff options
| -rw-r--r-- | app/models/ci/build.rb | 37 | ||||
| -rw-r--r-- | app/models/commit_status.rb | 18 | ||||
| -rw-r--r-- | features/steps/shared/builds.rb | 2 | ||||
| -rw-r--r-- | spec/features/pipelines_spec.rb | 11 | ||||
| -rw-r--r-- | spec/lib/ci/charts_spec.rb | 16 | ||||
| -rw-r--r-- | spec/models/ci/pipeline_spec.rb | 24 | ||||
| -rw-r--r-- | spec/requests/api/builds_spec.rb | 8 |
7 files changed, 60 insertions, 56 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 88a340379b8..92dad9377c9 100644 --- a/app/models/ci/build.rb +++ b/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 diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 64ce5431d63..522fa5d6911 100644 --- a/app/models/commit_status.rb +++ b/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 diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb index 5ed5cdb759f..70e6d4836b2 100644 --- a/features/steps/shared/builds.rb +++ b/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 diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb index 248e44a93aa..4e75f888176 100644 --- a/spec/features/pipelines_spec.rb +++ b/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 diff --git a/spec/lib/ci/charts_spec.rb b/spec/lib/ci/charts_spec.rb index 034ea098193..fb6cc398307 100644 --- a/spec/lib/ci/charts_spec.rb +++ b/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) + create(:ci_empty_pipeline, project: project, duration: nil) - chart = Ci::Charts::BuildTime.new(@pipeline.project) - expect(chart.build_times).to eq([2, 0]) + is_expected.to contain_exactly(2, 0) end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index eb762276cbe..adfe4bdd0c8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/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 } + 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 } - 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 + before do + build.skip + build2.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 + it 'matches sum of builds duration' do + expect(pipeline.reload.duration).to eq(build.duration + build2.duration) end end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 966d302dfd3..41503885dd9 100644 --- a/spec/requests/api/builds_spec.rb +++ b/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) |