Merge branch 'duration' into 'master'

Fix duration visualisation

https://dev.gitlab.org/gitlab/gitlab-ci/issues/174

https://github.com/gitlabhq/gitlab-ci/issues/565

See merge request !78

11 files changed, 86 insertions, 8 deletions

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 32d64fb..069005f 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -126,4 +126,25 @@ module ApplicationHelper
 
     [namespace, controller.controller_name, controller.action_name].compact.join(":")
   end
+
+  def duration_in_words(finished_at, started_at)
+    if finished_at && started_at
+      interval_in_seconds = finished_at.to_i - started_at.to_i
+    elsif started_at
+      interval_in_seconds = Time.now.to_i - started_at.to_i
+    end
+
+    time_interval_in_words(interval_in_seconds)
+  end
+
+  def time_interval_in_words(interval_in_seconds)
+    minutes = interval_in_seconds / 60
+    seconds = interval_in_seconds - minutes * 60
+
+    if minutes >= 1
+      "#{pluralize(minutes, "minute")} #{pluralize(seconds, "second")}"
+    else
+      "#{pluralize(seconds, "second")}"
+    end
+  end
 end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 6817a9e..bfc7878 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -190,7 +190,7 @@ class Commit < ActiveRecord::Base
   end
 
   def finished_at
-    @finished_at ||= builds.order('finished_at ASC').first.try(:finished_at)
+    @finished_at ||= builds.order('finished_at DESC').first.try(:finished_at)
   end
 
   def coverage
diff --git a/app/views/admin/builds/_build.html.haml b/app/views/admin/builds/_build.html.haml
index f71581d..aa4d1a3 100644
--- a/app/views/admin/builds/_build.html.haml
+++ b/app/views/admin/builds/_build.html.haml
@@ -25,7 +25,7 @@
 
     %td.duration
       - if build.duration
-        #{distance_of_time_in_words build.duration}
+        #{duration_in_words(build.finished_at, build.started_at)}
 
     %td.timestamp
       - if build.finished_at
diff --git a/app/views/builds/_build.html.haml b/app/views/builds/_build.html.haml
index 03a2321..7328e58 100644
--- a/app/views/builds/_build.html.haml
+++ b/app/views/builds/_build.html.haml
@@ -27,7 +27,7 @@
 
   %td.duration
     - if build.duration
-      #{distance_of_time_in_words build.duration}
+      #{duration_in_words(build.finished_at, build.started_at)}
 
   %td.timestamp
     - if build.finished_at
diff --git a/app/views/builds/show.html.haml b/app/views/builds/show.html.haml
index e7d73e0..d26bee0 100644
--- a/app/views/builds/show.html.haml
+++ b/app/views/builds/show.html.haml
@@ -52,7 +52,7 @@
           .pull-right
             %span
               %i.icon-time
-              #{distance_of_time_in_words @build.duration}
+              #{duration_in_words(@build.finished_at, @build.started_at)}
 
       .clearfix
         = @build.status
@@ -99,7 +99,7 @@
       - if @build.duration
         %p
           %span.attr-name Duration:
-          #{distance_of_time_in_words @build.duration}
+          #{duration_in_words(@build.finished_at, @build.started_at)}
       %p
         %span.attr-name Created:
         #{time_ago_in_words(@build.created_at)} ago
diff --git a/app/views/commits/_commit.html.haml b/app/views/commits/_commit.html.haml
index 54acf08..e5068d2 100644
--- a/app/views/commits/_commit.html.haml
+++ b/app/views/commits/_commit.html.haml
@@ -16,7 +16,7 @@
 
   %td.duration
     - if commit.duration > 0
-      #{distance_of_time_in_words commit.duration}
+      #{time_interval_in_words commit.duration}
 
   %td.timestamp
     - if commit.finished_at
diff --git a/app/views/commits/show.html.haml b/app/views/commits/show.html.haml
index 578f518..2d9e561 100644
--- a/app/views/commits/show.html.haml
+++ b/app/views/commits/show.html.haml
@@ -51,7 +51,7 @@
   - if @commit.duration > 0
     %small.pull-right
       %i.icon-time
-      #{distance_of_time_in_words @commit.duration}
+      #{time_interval_in_words @commit.duration}
 
 %table.builds
   %thead
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index a46b3d8..95858d1 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -39,7 +39,7 @@
       %th Commit
       %th Message
       %th Branch
-      %th Duration
+      %th Total duration
       %th Finished at
       - if @project.coverage_enabled?
         %th Coverage
diff --git a/spec/factories/builds.rb b/spec/factories/builds.rb
index a722a05..2aa6c03 100644
--- a/spec/factories/builds.rb
+++ b/spec/factories/builds.rb
@@ -24,5 +24,10 @@ FactoryGirl.define do
     started_at 'Di 29. Okt 09:51:28 CET 2013'
     finished_at 'Di 29. Okt 09:53:28 CET 2013'
     commands 'ls -a'
+
+    factory :not_started_build do
+      started_at nil
+      finished_at nil
+    end
   end
 end
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index bc6aa3b..984becf 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -54,4 +54,38 @@ describe ApplicationHelper do
       end
     end
   end
+
+  describe "#duration_in_words" do
+    it "returns minutes and seconds" do
+      intervals_in_words = {
+        100 => "1 minute 40 seconds",
+        121 => "2 minutes 1 second",
+        3721 => "62 minutes 1 second",
+        0 => "0 seconds"
+      }
+
+      intervals_in_words.each do |interval, expectation|
+        duration_in_words(Time.now + interval, Time.now).should == expectation
+      end
+    end
+
+    it "calculates interval from now if there is no finished_at" do
+      duration_in_words(nil, Time.now - 5).should == "5 seconds"
+    end
+  end
+
+  describe "#time_interval_in_words" do
+    it "returns minutes and seconds" do
+      intervals_in_words = {
+        100 => "1 minute 40 seconds",
+        121 => "2 minutes 1 second",
+        3721 => "62 minutes 1 second",
+        0 => "0 seconds"
+      }
+
+      intervals_in_words.each do |interval, expectation|
+        time_interval_in_words(interval).should == expectation
+      end
+    end
+  end
 end
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 03bc1e8..af53df9 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -161,4 +161,22 @@ describe Commit do
       commit.builds.size.should == 1
     end
   end
+
+  describe "#finished_at" do
+    let(:project) { FactoryGirl.create :project }
+    let(:commit) { FactoryGirl.create :commit, project: project }
+
+    it "returns finished_at of latest build" do
+      build = FactoryGirl.create :build, commit: commit, finished_at: Time.now - 60
+      build1 = FactoryGirl.create :build, commit: commit, finished_at: Time.now - 120
+
+      commit.finished_at.to_i.should == build.finished_at.to_i
+    end
+
+    it "returns nil if there is no finished build" do
+      build = FactoryGirl.create :not_started_build, commit: commit
+
+      commit.finished_at.should be_nil
+    end
+  end
 end