Fix specs

app/controllers/projects/commit_controller.rb

@@ -38,13 +38,13 @@ class Projects::CommitController < Projects::ApplicationController
   end
   def cancel_builds
-    ci_commit.builds.running_or_pending.each(&:cancel)
+    ci_builds.running_or_pending.each(&:cancel)
     redirect_back_or_default default: builds_namespace_project_commit_path(project.namespace, project, commit.sha)
   end
   def retry_builds
-    ci_commit.builds.latest.failed.each do |build|
+    ci_builds.latest.failed.each do |build|
       if build.retryable?
         Ci::Build.retry(build)
       end
@@ -98,6 +98,10 @@ class Projects::CommitController < Projects::ApplicationController
     @ci_commits ||= project.ci_commits.where(sha: commit.sha)
   end
+  def ci_builds
+    @ci_builds ||= Ci::Build.where(commit: ci_commits)
+  end
+
   def define_show_vars
     return git_not_found! unless commit

app/helpers/ci_status_helper.rb

@@ -34,6 +34,8 @@ module CiStatusHelper
   end
   def render_ci_status(ci_commit, tooltip_placement: 'auto left')
+    return unless ci_commit.is_a?(Commit) || ci_commit.is_a?(Ci::Commit)
+
     link_to ci_icon_for_status(ci_commit.status),
       project_ci_commit_path(ci_commit.project, ci_commit),
       class: "ci-status-link ci-status-icon-#{ci_commit.status.dasherize}",

app/models/ci/commit.rb

@@ -35,6 +35,11 @@ module Ci
     before_save :finished_at
     before_save :duration
+    # Invalidate object and save if when touched
+    after_touch :reload
+    after_touch :invalidate
+    after_touch :save
+
     def self.truncate_sha(sha)
       sha[0...8]
     end
@@ -86,9 +91,10 @@ module Ci
     end
     def invalidate
-      status = nil
-      started_at = nil
-      finished_at = nil
+      write_attribute(:status, nil)
+      write_attribute(:started_at, nil)
+      write_attribute(:finished_at, nil)
+      write_attribute(:duration, nil)
     end
     def create_builds(user, trigger_request = nil)
@@ -183,18 +189,18 @@ module Ci
         if yaml_errors.present?
           'failed'
         else
-          latest.status
+          latest.status || 'skipped'
         end
     end
     def update_started_at
       started_at =
-        statuses.order(:id).first.try(:started_at)
+        statuses.minimum(:started_at)
     end
     def update_finished_at
       finished_at =
-        statuses.order(id: :desc).first.try(:finished_at)
+        statuses.maximum(:finished_at)
     end
     def update_duration
@@ -204,9 +210,18 @@ module Ci
       end
     end
+    def update_statuses
+      update_status
+      update_started_at
+      update_finished_at
+      update_duration
+      save
+    end
+
     def save_yaml_error(error)
       return if self.yaml_errors?
       self.yaml_errors = error
+      update_status
       save
     end
   end

app/models/commit_status.rb

@@ -38,7 +38,7 @@ class CommitStatus < ActiveRecord::Base
   self.table_name = 'ci_builds'
   belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id
-  belongs_to :commit, class_name: 'Ci::Commit'
+  belongs_to :commit, class_name: 'Ci::Commit', touch: true
   belongs_to :user
   validates :commit, presence: true
@@ -47,7 +47,7 @@ class CommitStatus < ActiveRecord::Base
   alias_attribute :author, :user
-  scope :latest, -> { where(id: unscope(:select).select('max(id)').group(:name)) }
+  scope :latest, -> { where(id: unscope(:select).select('max(id)').group(:name, :commit_id)) }
   scope :ordered, -> { order(:ref, :stage_idx, :name) }
   AVAILABLE_STATUSES = ['pending', 'running', 'success', 'failed', 'canceled']
@@ -80,11 +80,6 @@ class CommitStatus < ActiveRecord::Base
     after_transition [:pending, :running] => :success do |commit_status|
       MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.commit.project, nil).trigger(commit_status)
     end
-
-    after_transition any => any do |commit_status|
-      commit_status.commit.invalidate
-      commit_status.save
-    end
   end
   delegate :before_sha, :sha, :short_sha, to: :commit, prefix: false

app/models/concerns/ci_status.rb

@@ -26,7 +26,7 @@ module CiStatus
   end
   included do
-    validates :status, inclusion: { in: %w(pending running failed success canceled) }
+    validates :status, inclusion: { in: %w(pending running failed success canceled skipped) }
     state_machine :status, initial: :pending do
       state :pending, value: 'pending'
@@ -34,6 +34,7 @@ module CiStatus
       state :failed, value: 'failed'
       state :success, value: 'success'
       state :canceled, value: 'canceled'
+      state :skipped, value: 'skipped'
     end
     scope :running, -> { where(status: 'running') }

app/services/ci/create_builds_service.rb

@@ -21,7 +21,7 @@ module Ci
       builds_attrs.map do |build_attrs|
         # don't create the same build twice
-        unless commit.builds.find_by(ref: @commit.ref, tag: @commit.tag,
+        unless @commit.builds.find_by(ref: @commit.ref, tag: @commit.tag,
                                      trigger_request: trigger_request, name: build_attrs[:name])
           build_attrs.slice!(:name,
                              :commands,

app/services/ci/create_trigger_request_service.rb

@@ -7,14 +7,14 @@ module Ci
       # check if ref is tag
       tag = project.repository.find_tag(ref).present?
-      ci_commit = project.ci_commits.create(commit.sha, ref)
+      ci_commit = project.ci_commits.create(sha: commit.sha, ref: ref, tag: tag)
       trigger_request = trigger.trigger_requests.create!(
         variables: variables,
         commit: ci_commit,
       )
-      if ci_commit.create_builds(ref, tag, nil, trigger_request)
+      if ci_commit.create_builds(nil, trigger_request)
         trigger_request
       end
     end

app/services/ci/image_for_build_service.rb

@@ -3,8 +3,9 @@ module Ci
     def execute(project, opts)
       sha = opts[:sha] || ref_sha(project, opts[:ref])
-      commit = project.ci_commits.find_by(sha: sha)
-      image_name = image_for_commit(commit)
+      ci_commits = project.ci_commits.where(sha: sha)
+      ci_commits = ci_commits.where(ref: opts[:ref]) if opts[:ref]
+      image_name = image_for_status(ci_commits.status)
       image_path = Rails.root.join('public/ci', image_name)
       OpenStruct.new(path: image_path, name: image_name)
@@ -16,9 +17,9 @@ module Ci
       project.commit(ref).try(:sha) if ref
     end
-    def image_for_commit(commit)
-      return 'build-unknown.svg' unless commit
-      'build-' + commit.status + ".svg"
+    def image_for_status(status)
+      status ||= 'unknown'
+      'build-' + status + ".svg"
     end
   end
 end

app/services/create_commit_builds_service.rb

@@ -37,6 +37,7 @@ class CreateCommitBuildsService
       commit.create_builds(user)
     end
+    commit.touch
     commit
   end
 end

app/views/projects/builds/show.html.haml

@@ -196,7 +196,7 @@
         .build-widget
           %h4.title #{pluralize(@builds.count(:id), "other build")} for
           = succeed ":" do
-            = link_to @build.commit.short_sha, builds_namespace_project_commit_path(@project.namespace, @project, build.sha), class: "monospace"
+            = link_to @build.commit.short_sha, builds_namespace_project_commit_path(@project.namespace, @project, @build.sha), class: "monospace"
           %table.table.builds
             - @builds.each_with_index do |build, i|
               %tr.build

app/views/shared/projects/_project.html.haml

@@ -7,7 +7,7 @@
 - show_last_commit_as_description = false unless local_assigns[:show_last_commit_as_description] == true && project.commit
 - css_class += " no-description" if project.description.blank? && !show_last_commit_as_description
 - cache_key = [project.namespace, project, controller.controller_name, controller.action_name, current_application_settings, 'v2.3']
-- cache_key.push(project.commit.status) if project.commit.status
+- cache_key.push(project.commit.status) if project.commit.try(:status)
 %li.project-row{ class: css_class }
   = cache(cache_key) do
@@ -15,7 +15,7 @@
       - if project.main_language
         %span
           = project.main_language
-      - if project.commit.status
+      - if project.commit.try(:status)
         %span
           = render_ci_status(project.commit)
       - if forks

db/fixtures/development/14_builds.rb

@@ -19,7 +19,7 @@ class Gitlab::Seeder::Builds
     commits = @project.repository.commits('master', nil, 5)
     commits_sha = commits.map { |commit| commit.raw.id }
     commits_sha.map do |sha|
-      @project.ensure_ci_commit(sha)
+      @project.ensure_ci_commit(sha, 'master')
     end
   rescue
     []

features/steps/project/merge_requests.rb

@@ -515,7 +515,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   step '"Bug NS-05" has CI status' do
     project = merge_request.source_project
     project.enable_ci
-    ci_commit = create :ci_commit, project: project, sha: merge_request.last_commit.id
+    ci_commit = create :ci_commit, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch
     create :ci_build, commit: ci_commit
   end

features/steps/shared/project.rb

@@ -230,7 +230,7 @@ module SharedProject
   step 'project "Shop" has CI build' do
     project = Project.find_by(name: "Shop")
-    create :ci_commit, project: project, sha: project.commit.sha
+    create :ci_commit, project: project, sha: project.commit.sha, ref: 'master'
   end
   step 'I should see last commit with CI status' do

lib/api/commit_statuses.rb

@@ -21,10 +21,9 @@ module API
         authorize!(:read_commit_status, user_project)
         not_found!('Commit') unless user_project.commit(params[:sha])
-        ci_commit = user_project.ci_commit(params[:sha], params[:ref])
-        return [] unless ci_commit
-        statuses = ci_commit.statuses
+        ci_commits = user_project.ci_commits.where(sha: params[:sha])
+        statuses = ::CommitStatus.where(commit: ci_commits)
         statuses = statuses.latest unless parse_boolean(params[:all])
         statuses = statuses.where(ref: params[:ref]) if params[:ref].present?
         statuses = statuses.where(stage: params[:stage]) if params[:stage].present?
@@ -51,7 +50,14 @@ module API
         commit = @project.commit(params[:sha])
         not_found! 'Commit' unless commit
-        ci_commit = @project.ensure_ci_commit(commit.sha)
+        ref = params[:ref] ||
+          begin
+            branches = @project.repository.branch_names_contains(commit.sha)
+            not_found! 'Reference for commit' if branches.none?
+            branches.first
+          end
+
+        ci_commit = @project.ensure_ci_commit(commit.sha, ref)
         name = params[:name] || params[:context]
         status = GenericCommitStatus.running_or_pending.find_by(commit: ci_commit, name: name, ref: params[:ref])

spec/features/commits_spec.rb

@@ -162,4 +162,9 @@ describe 'Commits' do
       end
     end
   end
+
+  def ci_status_path(ci_commit)
+    project = ci_commit.project
+    builds_namespace_project_commit_path(project.namespace, project, ci_commit.sha)
+  end
 end

spec/helpers/ci_status_helper_spec.rb

@@ -7,7 +7,7 @@ describe CiStatusHelper do
   let(:failed_commit) { double("Ci::Commit", status: 'failed') }
   describe 'ci_status_icon' do
-    it { expect(helper.ci_status_icon(success_commit)).to include('fa-check') }
-    it { expect(helper.ci_status_icon(failed_commit)).to include('fa-close') }
+    it { expect(helper.ci_icon_for_status(success_commit.status)).to include('fa-check') }
+    it { expect(helper.ci_icon_for_status(failed_commit.status)).to include('fa-close') }
   end
 end

spec/lib/gitlab/badge/build_spec.rb

@@ -42,7 +42,7 @@ describe Gitlab::Badge::Build do
   end
   context 'build exists' do
-    let(:ci_commit) { create(:ci_commit, project: project, sha: sha) }
+    let(:ci_commit) { create(:ci_commit, project: project, sha: sha, ref: branch) }
     let!(:build) { create(:ci_build, commit: ci_commit) }
@@ -57,7 +57,7 @@ describe Gitlab::Badge::Build do
       describe '#data' do
         let(:data) { badge.data }
-        it 'contains infromation about success' do
+        it 'contains information about success' do
           expect(status_node(data, 'success')).to be_truthy
         end
       end
@@ -74,7 +74,7 @@ describe Gitlab::Badge::Build do
       describe '#data' do
         let(:data) { badge.data }
-        it 'contains infromation about failure' do
+        it 'contains information about failure' do
           expect(status_node(data, 'failed')).to be_truthy
         end
       end

spec/models/ci/commit_spec.rb

@@ -52,57 +52,9 @@ describe Ci::Commit, models: true do
     it { expect(commit.sha).to start_with(subject) }
   end
-  describe :stage do
-    subject { commit.stage }
-
-    before do
-      @second = FactoryGirl.create :commit_status, commit: commit, name: 'deploy', stage: 'deploy', stage_idx: 1, status: 'pending'
-      @first = FactoryGirl.create :commit_status, commit: commit, name: 'test', stage: 'test', stage_idx: 0, status: 'pending'
-    end
-
-    it 'returns first running stage' do
-      is_expected.to eq('test')
-    end
-
-    context 'first build succeeded' do
-      before do
-        @first.success
-      end
-
-      it 'returns last running stage' do
-        is_expected.to eq('deploy')
-      end
-    end
-
-    context 'all builds succeeded' do
-      before do
-        @first.success
-        @second.success
-      end
-
-      it 'returns nil' do
-        is_expected.to be_nil
-      end
-    end
-  end
-
   describe :create_next_builds do
   end
-  describe :refs do
-    subject { commit.refs }
-
-    before do
-      FactoryGirl.create :commit_status, commit: commit, name: 'deploy'
-      FactoryGirl.create :commit_status, commit: commit, name: 'deploy', ref: 'develop'
-      FactoryGirl.create :commit_status, commit: commit, name: 'deploy', ref: 'master'
-    end
-
-    it 'returns all refs' do
-      is_expected.to contain_exactly('master', 'develop', nil)
-    end
-  end
-
   describe :retried do
     subject { commit.retried }
@@ -117,10 +69,10 @@ describe Ci::Commit, models: true do
   end
   describe :create_builds do
-    let!(:commit) { FactoryGirl.create :ci_commit, project: project }
+    let!(:commit) { FactoryGirl.create :ci_commit, project: project, ref: 'master', tag: false }
     def create_builds(trigger_request = nil)
-      commit.create_builds('master', false, nil, trigger_request)
+      commit.create_builds(nil, trigger_request)
     end
     def create_next_builds
@@ -143,67 +95,6 @@ describe Ci::Commit, models: true do
       expect(create_next_builds).to be_falsey
     end
-    context 'for different ref' do
-      def create_develop_builds
-        commit.create_builds('develop', false, nil, nil)
-      end
-
-      it 'creates builds' do
-        expect(create_builds).to be_truthy
-        commit.builds.update_all(status: "success")
-        expect(commit.builds.count(:all)).to eq(2)
-
-        expect(create_develop_builds).to be_truthy
-        commit.builds.update_all(status: "success")
-        expect(commit.builds.count(:all)).to eq(4)
-        expect(commit.refs.size).to eq(2)
-        expect(commit.builds.pluck(:name).uniq.size).to eq(2)
-      end
-    end
-
-    context 'for build triggers' do
-      let(:trigger) { FactoryGirl.create :ci_trigger, project: project }
-      let(:trigger_request) { FactoryGirl.create :ci_trigger_request, commit: commit, trigger: trigger }
-
-      it 'creates builds' do
-        expect(create_builds(trigger_request)).to be_truthy
-        expect(commit.builds.count(:all)).to eq(2)
-      end
-
-      it 'rebuilds commit' do
-        expect(create_builds).to be_truthy
-        expect(commit.builds.count(:all)).to eq(2)
-
-        expect(create_builds(trigger_request)).to be_truthy
-        expect(commit.builds.count(:all)).to eq(4)
-      end
-
-      it 'creates next builds' do
-        expect(create_builds(trigger_request)).to be_truthy
-        expect(commit.builds.count(:all)).to eq(2)
-        commit.builds.update_all(status: "success")
-
-        expect(create_next_builds).to be_truthy
-        expect(commit.builds.count(:all)).to eq(4)
-      end
-
-      context 'for [ci skip]' do
-        before do
-          allow(commit).to receive(:git_commit_message) { 'message [ci skip]' }
-        end
-
-        it 'rebuilds commit' do
-          expect(commit.status).to eq('skipped')
-          expect(create_builds).to be_truthy
-
-          # since everything in Ci::Commit is cached we need to fetch a new object
-          new_commit = Ci::Commit.find_by_id(commit.id)
-          expect(new_commit.status).to eq('pending')
-        end
-      end
-    end
-
-
     context 'custom stage with first job allowed to fail' do
       let(:yaml) do
         {
@@ -284,6 +175,7 @@ describe Ci::Commit, models: true do
         commit.builds.running_or_pending.each(&:success)
         expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success')
+        commit.reload
         expect(commit.status).to eq('success')
       end
@@ -306,6 +198,7 @@ describe Ci::Commit, models: true do
         commit.builds.running_or_pending.each(&:success)
         expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success')
+        commit.reload
         expect(commit.status).to eq('failed')
       end
@@ -329,6 +222,7 @@ describe Ci::Commit, models: true do
         expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
         expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success')
+        commit.reload
         expect(commit.status).to eq('failed')
       end
@@ -351,6 +245,7 @@ describe Ci::Commit, models: true do
         commit.builds.running_or_pending.each(&:success)
         expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success')
+        commit.reload
         expect(commit.status).to eq('failed')
       end
     end

spec/models/commit_status_spec.rb

@@ -163,21 +163,7 @@ describe CommitStatus, models: true do
     end
     it 'return unique statuses' do
-      is_expected.to eq([@commit2, @commit3, @commit4, @commit5])
-    end
-  end
-
-  describe :for_ref do
-    subject { CommitStatus.for_ref('bb').order(:id) }
-
-    before do
-      @commit1 = FactoryGirl.create :commit_status, commit: commit, name: 'aa', ref: 'bb', status: 'running'
-      @commit2 = FactoryGirl.create :commit_status, commit: commit, name: 'cc', ref: 'cc', status: 'pending'
-      @commit3 = FactoryGirl.create :commit_status, commit: commit, name: 'aa', ref: nil, status: 'success'
-    end
-
-    it 'return statuses with equal and nil ref set' do
-      is_expected.to eq([@commit1])
+      is_expected.to eq([@commit4, @commit5])
     end
   end