diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 9799c06c19f..b7d4b492a75 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -260,13 +260,8 @@ module Ci def update_duration return unless started_at - calculated_status = %w[success failed running canceled] - calculated_builds = builds.latest.where(status: calculated_status) - calculator = Gitlab::Ci::PipelineDuration.from_builds(calculated_builds) - - self.duration = calculator.duration - self.pending_duration = - started_at - created_at + calculator.pending_duration + self.duration, self.pending_duration = + Gitlab::Ci::PipelineDuration.from_pipeline(self) end def execute_hooks diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb index e37ba19bca9..baf84954b7e 100644 --- a/lib/gitlab/ci/pipeline_duration.rb +++ b/lib/gitlab/ci/pipeline_duration.rb @@ -106,17 +106,27 @@ module Gitlab end end - def self.from_builds(builds) + def self.from_pipeline(pipeline) + status = %w[success failed running canceled] + builds = pipeline.builds.latest.where(status: status) + + duration = from_builds(builds, :started_at, :finished_at).duration + queued = from_builds(builds, :queued_at, :started_at).duration + + [duration, pipeline.started_at - pipeline.created_at + queued] + end + + def self.from_builds(builds, from, to) now = Time.now periods = builds.map do |b| - Period.new(b.started_at || now, b.finished_at || now) + Period.new(b.public_send(from) || now, b.public_send(to) || now) end new(periods) end - attr_reader :duration, :pending_duration + attr_reader :duration def initialize(periods) process(periods.sort_by(&:first)) @@ -125,10 +135,7 @@ module Gitlab private def process(periods) - merged = process_periods(periods) - - @duration = process_duration(merged) - @pending_duration = process_pending_duration(merged) + @duration = process_duration(process_periods(periods)) end def process_periods(periods) @@ -157,13 +164,6 @@ module Gitlab result + per.duration end end - - def process_pending_duration(periods) - return 0 if periods.empty? - - total = periods.last.last - periods.first.first - total - duration - end end end end diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb index acb690581d6..a160c0a6e39 100644 --- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb +++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb @@ -6,7 +6,6 @@ describe Gitlab::Ci::PipelineDuration do shared_examples 'calculating duration' do it do expect(calculator.duration).to eq(duration) - expect(calculator.pending_duration).to eq(pending_duration) end end @@ -19,7 +18,6 @@ describe Gitlab::Ci::PipelineDuration do end let(:duration) { 4 } - let(:pending_duration) { 2 } it_behaves_like 'calculating duration' end @@ -34,7 +32,6 @@ describe Gitlab::Ci::PipelineDuration do end let(:duration) { 4 } - let(:pending_duration) { 0 } it_behaves_like 'calculating duration' end @@ -48,7 +45,6 @@ describe Gitlab::Ci::PipelineDuration do end let(:duration) { 8 } - let(:pending_duration) { 1 } it_behaves_like 'calculating duration' end @@ -62,7 +58,6 @@ describe Gitlab::Ci::PipelineDuration do end let(:duration) { 4 } - let(:pending_duration) { 3 } it_behaves_like 'calculating duration' end @@ -80,7 +75,6 @@ describe Gitlab::Ci::PipelineDuration do end let(:duration) { 7 } - let(:pending_duration) { 2 } it_behaves_like 'calculating duration' end @@ -95,7 +89,6 @@ describe Gitlab::Ci::PipelineDuration do end let(:duration) { 6 } - let(:pending_duration) { 1 } it_behaves_like 'calculating duration' end @@ -108,7 +101,6 @@ describe Gitlab::Ci::PipelineDuration do end let(:duration) { 4 } - let(:pending_duration) { 2 } it_behaves_like 'calculating duration' end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index a34b9cb4461..2d239caab64 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -124,18 +124,23 @@ describe Ci::Pipeline, models: true do describe 'state machine' do let(:current) { Time.now.change(usec: 0) } - let(:build) { create_build('build1', current - 60) } - let(:another_build) { create_build('build2', current + 20) } + let(:build) { create_build('build1', current, 10) } + let(:another_build) { create_build('build2', current + 80, 5) } - describe '#duration' do + describe '#duration and #pending_duration' do before do - pipeline.run + pipeline.update(created_at: current) - travel_to(current) do + travel_to(current + 5) do + pipeline.run + pipeline.save + end + + travel_to(current + 70) do build.success end - travel_to(current + 80) do + travel_to(current + 145) do another_build.drop end @@ -143,7 +148,10 @@ describe Ci::Pipeline, models: true do end it 'matches sum of builds duration' do - expect(pipeline.reload.duration).to eq(120) + pipeline.reload + + expect(pipeline.duration).to eq(70 - 10 + 145 - 85) + expect(pipeline.pending_duration).to eq(5 + 10 + 5) end end @@ -175,11 +183,12 @@ describe Ci::Pipeline, models: true do end end - def create_build(name, started_at = current) + def create_build(name, queued_at = current, started_from = 0) create(:ci_build, name: name, pipeline: pipeline, - started_at: started_at) + queued_at: queued_at, + started_at: queued_at + started_from) end end