Write specs for this feature

7 files changed, 188 insertions, 19 deletions

diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb
index 412ab44aaf6..f2667e5476b 100644
--- a/app/models/ci/commit.rb
+++ b/app/models/ci/commit.rb
@@ -26,7 +26,10 @@ module Ci
     has_many :builds, class_name: 'Ci::Build'
     has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest'
 
+    delegate :stages, to: :statuses
+
     validates_presence_of :sha
+    validates_presence_of :status
     validate :valid_commit_sha
 
     # Invalidate object and save if when touched
@@ -40,10 +43,6 @@ module Ci
       CommitStatus.where(commit: all).stages
     end
 
-    def stages
-      statuses.stages
-    end
-
     def project_id
       project.id
     end
@@ -82,7 +81,7 @@ module Ci
 
     def retryable?
       builds.latest.any? do |build|
-        build.failed? || build.retryable?
+        build.failed? && build.retryable?
       end
     end
 
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 06d296eef08..24a26b4be8c 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -93,7 +93,10 @@ class CommitStatus < ActiveRecord::Base
   end
 
   def self.stages_status
-    Hash[group(:stage).pluck(:stage, self.status_sql)]
+    all.stages.inject({}) do |h, stage|
+      h[stage] = all.where(stage: stage).status
+      h
+    end
   end
 
   def ignored?
@@ -101,11 +104,13 @@ class CommitStatus < ActiveRecord::Base
   end
 
   def duration
-    if started_at && finished_at
-      finished_at - started_at
-    elsif started_at
-      Time.now - started_at
-    end
+    duration =
+      if started_at && finished_at
+        finished_at - started_at
+      elsif started_at
+        Time.now - started_at
+      end
+    duration.to_i
   end
 
   def stuck?
diff --git a/app/models/concerns/statuseable.rb b/app/models/concerns/statuseable.rb
index f34dca29120..7b0c9789fb9 100644
--- a/app/models/concerns/statuseable.rb
+++ b/app/models/concerns/statuseable.rb
@@ -33,7 +33,7 @@ module Statuseable
 
     def duration
       duration_array = all.map(&:duration).compact
-      duration_array.reduce(:+).to_i
+      duration_array.reduce(:+)
     end
 
     def started_at
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 0b52dd8a743..7388ed2f4ea 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -50,12 +50,19 @@ module API
         commit = @project.commit(params[:sha])
         not_found! 'Commit' unless commit
 
-        ref = params[:ref] ||
-          begin
-            branches = @project.repository.branch_names_contains(commit.sha)
-            not_found! 'Reference for commit' if branches.none?
-            branches.first
-          end
+        # Since the CommitStatus is attached to Ci::Commit (in the future Pipeline)
+        # We need to always have the pipeline object
+        # To have a valid pipeline object that can be attached to specific MR
+        # Other CI service needs to send `ref`
+        # If we don't receive it, we will attach the CommitStatus to
+        # the first found branch on that commit
+
+        ref = params[:ref]
+        unless ref
+          branches = @project.repository.branch_names_contains(commit.sha)
+          not_found! 'References for commit' if branches.none?
+          ref = branches.first
+        end
 
         ci_commit = @project.ensure_ci_commit(commit.sha, ref)
 
diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb
index fb3b61ad7c7..aef4f007202 100644
--- a/spec/models/ci/commit_spec.rb
+++ b/spec/models/ci/commit_spec.rb
@@ -27,6 +27,8 @@ describe Ci::Commit, models: true do
   it { is_expected.to have_many(:trigger_requests) }
   it { is_expected.to have_many(:builds) }
   it { is_expected.to validate_presence_of :sha }
+  it { is_expected.to validate_presence_of :status }
+  it { is_expected.to delegate_method(:stages).to(:statuses) }
 
   it { is_expected.to respond_to :git_author_name }
   it { is_expected.to respond_to :git_author_email }
@@ -297,4 +299,73 @@ describe Ci::Commit, models: true do
       expect(commit.coverage).to be_nil
     end
   end
+
+  describe '#retryable?' do
+    subject { commit.retryable? }
+
+    context 'no failed builds' do
+      before do
+        FactoryGirl.create :ci_build, name: "rspec", commit: commit, status: 'success'
+      end
+
+      it 'be not retryable' do
+        is_expected.to be_falsey
+      end
+    end
+
+    context 'with failed builds' do
+      before do
+        FactoryGirl.create :ci_build, name: "rspec", commit: commit, status: 'running'
+        FactoryGirl.create :ci_build, name: "rubocop", commit: commit, status: 'failed'
+      end
+
+      it 'be retryable' do
+        is_expected.to be_truthy
+      end
+    end
+  end
+
+  describe '#stages' do
+    let(:commit2) { FactoryGirl.create :ci_commit, project: project }
+    subject { CommitStatus.where(commit: [commit, commit2]).stages }
+
+    before do
+      FactoryGirl.create :ci_build, commit: commit2, stage: 'test', stage_idx: 1
+      FactoryGirl.create :ci_build, commit: commit, stage: 'build', stage_idx: 0
+    end
+
+    it 'return all stages' do
+      is_expected.to eq(%w(build test))
+    end
+  end
+
+  describe '#update_state' do
+    it 'execute update_state after touching object' do
+      expect(commit).to receive(:update_state).and_return(true)
+      commit.touch
+    end
+
+    context 'dependent objects' do
+      let(:commit_status) { build :commit_status, commit: commit }
+
+      it 'execute update_state after saving dependent object' do
+        expect(commit).to receive(:update_state).and_return(true)
+        commit_status.save
+      end
+    end
+
+    context 'update state' do
+      let(:build) { FactoryGirl.create :ci_build, :success, commit: commit, started_at: Time.now - 120, finished_at: Time.now - 60 }
+
+      before do
+        build
+      end
+
+      [:status, :started_at, :finished_at, :duration].each do |param|
+        it "update #{param}" do
+          expect(commit.send(param)).to eq(build.send(param))
+        end
+      end
+    end
+  end
 end
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index 1c64947f1f5..31d546820c2 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -182,4 +182,78 @@ describe CommitStatus, models: true do
       is_expected.to eq([@commit1, @commit2])
     end
   end
+
+  describe '#before_sha' do
+    subject { commit_status.before_sha }
+
+    context 'when no before_sha is set for ci::commit' do
+      before { commit.before_sha = nil }
+
+      it 'return blank sha' do
+        is_expected.to eq(Gitlab::Git::BLANK_SHA)
+      end
+    end
+
+    context 'for before_sha set for ci::commit' do
+      let(:value) { '1234' }
+      before { commit.before_sha = value }
+
+      it 'return the set value' do
+        is_expected.to eq(value)
+      end
+    end
+  end
+
+  describe '#stages' do
+    before do
+      FactoryGirl.create :commit_status, commit: commit, stage: 'build', stage_idx: 0, status: 'success'
+      FactoryGirl.create :commit_status, commit: commit, stage: 'build', stage_idx: 0, status: 'failed'
+      FactoryGirl.create :commit_status, commit: commit, stage: 'deploy', stage_idx: 2, status: 'running'
+      FactoryGirl.create :commit_status, commit: commit, stage: 'test', stage_idx: 1, status: 'success'
+    end
+
+    context 'stages list' do
+      subject { CommitStatus.where(commit: commit).stages }
+
+      it 'return ordered list of stages' do
+        is_expected.to eq(%w(build test deploy))
+      end
+    end
+
+    context 'stages with statuses' do
+      subject { CommitStatus.where(commit: commit).stages_status }
+
+      it 'return list of stages with statuses' do
+        is_expected.to eq({
+          'build' => 'failed',
+          'test' => 'success',
+          'deploy' => 'running'
+        })
+      end
+    end
+  end
+
+  describe '#branch?' do
+    subject { commit_status.branch? }
+
+    context 'is not a tag' do
+      before do
+        commit_status.tag = false
+      end
+
+      it 'return true when tag is set to false' do
+        is_expected.to be_truthy
+      end
+    end
+
+    context 'is not a tag' do
+      before do
+        commit_status.tag = true
+      end
+
+      it 'return false when tag is set to true' do
+        is_expected.to be_falsey
+      end
+    end
+  end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 1688e91ca62..becc743de31 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -443,7 +443,20 @@ describe Project, models: true do
     let(:project) { create :project }
     let(:commit) { create :ci_commit, project: project, ref: 'master' }
 
-    it { expect(project.ci_commit(commit.sha, 'master')).to eq(commit) }
+    subject { project.ci_commit(commit.sha, 'master') }
+
+    it { is_expected.to eq(commit) }
+
+    context 'return latest' do
+      let(:commit2) { create :ci_commit, project: project, ref: 'master' }
+
+      before do
+        commit
+        commit2
+      end
+
+      it { is_expected.to eq(commit2) }
+    end
   end
 
   describe :builds_enabled do