From defd8899056c26593939243b05c365605ac06fdc Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 2 Sep 2016 21:08:26 +0800
Subject: [PATCH] Actually we still need to use total - running to get:

real pending time, because that's actually by definition,
(the time that it's not running!)
or we could end up with awfully complicated algorithm :(
---
 lib/gitlab/ci/pipeline_duration.rb |  7 ++++---
 spec/models/ci/pipeline_spec.rb    | 17 +++++++++++------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index baf84954b7e..0c7c38e2b67 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -110,10 +110,11 @@ module Gitlab
         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
+        running = from_builds(builds, :started_at, :finished_at).duration
+        total = from_builds(builds, :queued_at, :finished_at).duration
+        pending = pipeline.started_at - pipeline.created_at
 
-        [duration, pipeline.started_at - pipeline.created_at + queued]
+        [running, pending + total - running]
       end
 
       def self.from_builds(builds, from, to)
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 68a88217afe..4615daa6551 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -125,7 +125,8 @@ describe Ci::Pipeline, models: true do
   describe 'state machine' do
     let(:current) { Time.now.change(usec: 0) }
     let(:build) { create_build('build1', current, 10) }
-    let(:another_build) { create_build('build2', current + 80, 5) }
+    let(:build_b) { create_build('build2', current, 20) }
+    let(:build_c) { create_build('build3', current + 40, 20) }
 
     describe '#duration and #pending_duration' do
       before do
@@ -136,12 +137,16 @@ describe Ci::Pipeline, models: true do
           pipeline.save
         end
 
-        travel_to(current + 70) do
+        travel_to(current + 30) do
           build.success
         end
 
-        travel_to(current + 145) do
-          another_build.drop
+        travel_to(current + 40) do
+          build_b.drop
+        end
+
+        travel_to(current + 70) do
+          build_c.success
         end
 
         pipeline.drop
@@ -150,8 +155,8 @@ describe Ci::Pipeline, models: true do
       it 'matches sum of builds duration' do
         pipeline.reload
 
-        expect(pipeline.duration).to eq(70 - 10 + 145 - 85)
-        expect(pipeline.pending_duration).to eq(5 + 10 + 5)
+        expect(pipeline.duration).to eq(40)
+        expect(pipeline.pending_duration).to eq(35)
       end
     end