Merge branch 'fix-commit-status-rendering' into 'master'

Cleanup Ci::Commit, Ci::Build and CommitStatus views

This MR tries to do first sweep of cleanups to Ci::Commit and
Ci::Build objects removing all view-related functions and fixing the
API from other side.

Fixes: https://gitlab.com/gitlab-org/gitlab-ce/issues/6046

See merge request !2760

app/models/ci/build.rb

@@ -37,8 +37,6 @@
 
 module Ci
   class Build < CommitStatus
-    include Gitlab::Application.routes.url_helpers
-
     LAZY_ATTRIBUTES = ['trace']
 
     belongs_to :runner, class_name: 'Ci::Runner'
@@ -128,7 +126,7 @@ module Ci
     end
 
     def retried?
-      !self.commit.latest_builds_for_ref(self.ref).include?(self)
+      !self.commit.latest_statuses_for_ref(self.ref).include?(self)
     end
 
     def depends_on_builds
@@ -309,22 +307,6 @@ module Ci
       project.valid_runners_token? token
     end
 
-    def target_url
-      namespace_project_build_url(project.namespace, project, self)
-    end
-
-    def cancel_url
-      if active?
-        cancel_namespace_project_build_path(project.namespace, project, self)
-      end
-    end
-
-    def retry_url
-      if retryable?
-        retry_namespace_project_build_path(project.namespace, project, self)
-      end
-    end
-
     def can_be_served?(runner)
       (tag_list - runner.tag_list).empty?
     end
@@ -333,7 +315,7 @@ module Ci
       project.any_runners? { |runner| runner.active? && runner.online? && can_be_served?(runner) }
     end
 
-    def show_warning?
+    def stuck?
       pending? && !any_runners_online?
     end
 
@@ -348,18 +330,6 @@ module Ci
       artifacts_file.exists?
     end
 
-    def artifacts_download_url
-      if artifacts?
-        download_namespace_project_build_artifacts_path(project.namespace, project, self)
-      end
-    end
-
-    def artifacts_browse_url
-      if artifacts_metadata?
-        browse_namespace_project_build_artifacts_path(project.namespace, project, self)
-      end
-    end
-
     def artifacts_metadata?
       artifacts? && artifacts_metadata.exists?
     end

app/models/ci/commit.rb

@@ -25,8 +25,6 @@ module Ci
     has_many :builds, class_name: 'Ci::Build'
     has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest'
 
-    scope :ordered, -> { order('CASE WHEN ci_commits.committed_at IS NULL THEN 0 ELSE 1 END', :committed_at, :id) }
-
     validates_presence_of :sha
     validate :valid_commit_sha
 
@@ -42,16 +40,6 @@ module Ci
       project.id
     end
 
-    def last_build
-      builds.order(:id).last
-    end
-
-    def retry
-      latest_builds.each do |build|
-        Ci::Build.retry(build)
-      end
-    end
-
     def valid_commit_sha
       if self.sha == Gitlab::Git::BLANK_SHA
         self.errors.add(:sha, " cant be 00000000 (branch removal)")
@@ -121,12 +109,14 @@ module Ci
       @latest_statuses ||= statuses.latest.to_a
     end
 
-    def latest_builds
-      @latest_builds ||= builds.latest.to_a
+    def latest_statuses_for_ref(ref)
+      latest_statuses.select { |status| status.ref == ref }
     end
 
-    def latest_builds_for_ref(ref)
-      latest_builds.select { |build| build.ref == ref }
+    def matrix_builds(build = nil)
+      matrix_builds = builds.latest.ordered
+      matrix_builds = matrix_builds.similar(build) if build
+      matrix_builds.to_a
     end
 
     def retried
@@ -170,7 +160,7 @@ module Ci
     end
 
     def duration
-      duration_array = latest_statuses.map(&:duration).compact
+      duration_array = statuses.map(&:duration).compact
       duration_array.reduce(:+).to_i
     end
 
@@ -183,16 +173,12 @@ module Ci
     end
 
     def coverage
-      coverage_array = latest_builds.map(&:coverage).compact
+      coverage_array = latest_statuses.map(&:coverage).compact
       if coverage_array.size >= 1
         '%.2f' % (coverage_array.reduce(:+) / coverage_array.size)
       end
     end
 
-    def matrix_for_ref?(ref)
-      latest_builds_for_ref(ref).size > 1
-    end
-
     def config_processor
       return nil unless ci_yaml_file
       @config_processor ||= Ci::GitlabCiYamlProcessor.new(ci_yaml_file, project.path_with_namespace)
@@ -218,10 +204,6 @@ module Ci
       git_commit_message =~ /(\[ci skip\])/ if git_commit_message
     end
 
-    def update_committed!
-      update!(committed_at: DateTime.now)
-    end
-
     private
 
     def save_yaml_error(error)

app/models/commit_status.rb

@@ -125,23 +125,7 @@ class CommitStatus < ActiveRecord::Base
     end
   end
 
-  def cancel_url
-    nil
-  end
-
-  def retry_url
-    nil
-  end
-
-  def show_warning?
+  def stuck?
     false
   end
-
-  def artifacts_download_url
-    nil
-  end
-
-  def artifacts_browse_url
-    nil
-  end
 end

app/services/ci/image_for_build_service.rb

@@ -3,7 +3,7 @@ module Ci
     def execute(project, opts)
       sha = opts[:sha] || ref_sha(project, opts[:ref])
 
-      commit = project.ci_commits.ordered.find_by(sha: sha)
+      commit = project.ci_commits.find_by(sha: sha)
       image_name = image_for_commit(commit)
 
       image_path = Rails.root.join('public/ci', image_name)

app/services/create_commit_builds_service.rb

@@ -33,7 +33,6 @@ class CreateCommitBuildsService
     unless commit.skip_ci?
       # Create builds for commit
       tag = Gitlab::Git.tag_ref?(origin_ref)
-      commit.update_committed!
       commit.create_builds(ref, tag, user)
     end
 

app/views/admin/builds/_build.html.haml

@@ -4,13 +4,13 @@
     = ci_status_with_icon(build.status)
 
   %td.build-link
-    - if can?(current_user, :read_build, project) && build.target_url
-      = link_to build.target_url do
+    - if can?(current_user, :read_build, build.project)
+      = link_to namespace_project_build_url(build.project.namespace, build.project, build) do
         %strong Build ##{build.id}
     - else
       %strong Build ##{build.id}
 
-    - if build.show_warning?
+    - if build.stuck?
       %i.fa.fa-warning.text-warning
 
   %td
@@ -18,11 +18,11 @@
       = link_to project.name_with_namespace, admin_namespace_project_path(project.namespace, project), class: "monospace"
 
   %td
-    = link_to build.short_sha, namespace_project_commit_path(project.namespace, project, build.sha), class: "monospace"
+    = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "monospace"
 
   %td
     - if build.ref
-      = link_to build.ref, namespace_project_commits_path(project.namespace, project, build.ref)
+      = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref)
     - else
       .light none
 
@@ -61,13 +61,12 @@
   %td
     .pull-right
       - if can?(current_user, :read_build, project) && build.artifacts?
-        = link_to build.artifacts_download_url, title: 'Download artifacts' do
+        = link_to download_namespace_project_build_artifacts_path(build.project.namespace, build.project, build), title: 'Download artifacts' do
           %i.fa.fa-download
       - if can?(current_user, :update_build, build.project)
         - if build.active?
-          - if build.cancel_url
-            = link_to build.cancel_url, method: :post, title: 'Cancel' do
+          = link_to cancel_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Cancel' do
             %i.fa.fa-remove.cred
-        - elsif defined?(allow_retry) && allow_retry && build.retry_url
-          = link_to build.retry_url, method: :post, title: 'Retry' do
+        - elsif defined?(allow_retry) && allow_retry && build.retryable?
+          = link_to retry_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Retry' do
             %i.fa.fa-repeat

app/views/ci/commits/_commit.html.haml

-%tr.build
-  %td.status
-    = ci_status_with_icon(commit.status)
-    - if commit.running?
-      ·
-      = commit.stage
-
-
-  %td.build-link
-    = link_to ci_status_path(commit) do
-      %strong #{commit.short_sha}
-
-  %td.build-message
-    %span= truncate_first_line(commit.git_commit_message)
-
-  %td.build-branch
-    - unless @ref
-      %span
-        - commit.refs.each do |ref|
-          = link_to truncate(ref, length: 25), ci_project_path(@project, ref: ref)
-
-  %td.duration
-    - if commit.duration > 0
-      #{time_interval_in_words commit.duration}
-
-  %td.timestamp
-    - if commit.finished_at
-      %span #{time_ago_in_words commit.finished_at} ago
-
-  - if commit.coverage
-    %td.coverage
-      #{commit.coverage}%

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

@@ -55,7 +55,6 @@
               %th Coverage
             %th
 
-        - @builds.each do |build|
-          = render 'projects/commit_statuses/commit_status', commit_status: build, commit_sha: true, stage: true, coverage: @project.build_coverage_enabled?, allow_retry: true
+        = render @builds, commit_sha: true, stage: true, allow_retry: true, coverage: @project.build_coverage_enabled?
 
     = paginate @builds, theme: 'gitlab'

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

@@ -13,9 +13,10 @@
       = link_to "merge request ##{merge_request.iid}", merge_request_path(merge_request)
 
   #up-build-trace
-  - if @commit.matrix_for_ref?(@build.ref)
+  - builds = @build.commit.matrix_builds(@build)
+  - if builds.size > 1
     %ul.nav-links.no-top.no-bottom
-      - @commit.latest_builds_for_ref(@build.ref).each do |build|
+      - builds.each do |build|
         %li{class: ('active' if build == @build) }
           = link_to namespace_project_build_path(@project.namespace, @project, build) do
             = ci_icon_for_status(build.status)
@@ -44,7 +45,7 @@
         .pull-right
           #{time_ago_with_tooltip(@build.finished_at) if @build.finished_at}
 
-  - if @build.show_warning?
+  - if @build.stuck?
     - unless @build.any_runners_online?
       .bs-callout.bs-callout-warning
         %p
@@ -100,12 +101,12 @@
           %h4.title Build artifacts
           .center
             .btn-group{ role: :group }
-              = link_to @build.artifacts_download_url, class: 'btn btn-sm btn-primary' do
+              = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-primary' do
                 = icon('download')
                 Download
 
               - if @build.artifacts_metadata?
-                = link_to @build.artifacts_browse_url, class: 'btn btn-sm btn-primary' do
+                = link_to browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-primary' do
                   = icon('folder-open')
                   Browse
 
@@ -115,10 +116,10 @@
           - if can?(current_user, :update_build, @project)
             .center
               .btn-group{ role: :group }
-                - if @build.cancel_url
-                  = link_to "Cancel", @build.cancel_url, class: 'btn btn-sm btn-danger', method: :post
-                - elsif @build.retry_url
-                  = link_to "Retry", @build.retry_url, class: 'btn btn-sm btn-primary', method: :post
+                - if @build.active?
+                  = link_to "Cancel", cancel_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-danger', method: :post
+                - elsif @build.retryable?
+                  = link_to "Retry", retry_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-primary', method: :post
 
                 - if @build.erasable?
                   = link_to erase_namespace_project_build_path(@project.namespace, @project, @build),

app/views/projects/ci/builds/_build.html.haml

+%tr.build
+  %td.status
+    - if can?(current_user, :read_build, build)
+      = link_to namespace_project_build_url(build.project.namespace, build.project, build), class: "ci-status ci-#{build.status}" do
+        = ci_status_with_icon(build.status)
+    - else
+      = ci_status_with_icon(build.status)
+
+  %td.build-link
+    - if can?(current_user, :read_build, build)
+      = link_to namespace_project_build_url(build.project.namespace, build.project, build) do
+        %strong ##{build.id}
+    - else
+      %strong ##{build.id}
+
+    - if build.stuck?
+      %i.fa.fa-warning.text-warning
+
+  - if defined?(commit_sha) && commit_sha
+    %td
+      = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "monospace"
+
+  %td
+    - if build.ref
+      = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref)
+    - else
+      .light none
+
+  - if defined?(runner) && runner
+    %td
+      - if build.try(:runner)
+        = runner_link(build.runner)
+      - else
+        .light none
+
+  - if defined?(stage) && stage
+    %td
+      = build.stage
+
+  %td
+    = build.name
+
+    .pull-right
+      - if build.tags.any?
+        - build.tags.each do |tag|
+          %span.label.label-primary
+            = tag
+      - if build.try(:trigger_request)
+        %span.label.label-info triggered
+      - if build.try(:allow_failure)
+        %span.label.label-danger allowed to fail
+
+  %td.duration
+    - if build.duration
+      #{duration_in_words(build.finished_at, build.started_at)}
+
+  %td.timestamp
+    - if build.finished_at
+      %span #{time_ago_with_tooltip(build.finished_at)}
+
+  - if defined?(coverage) && coverage
+    %td.coverage
+      - if build.try(:coverage)
+        #{build.coverage}%
+
+  %td
+    .pull-right
+      - if can?(current_user, :read_build, build) && build.artifacts?
+        = link_to download_namespace_project_build_artifacts_path(build.project.namespace, build.project, build), title: 'Download artifacts' do
+          %i.fa.fa-download
+      - if can?(current_user, :update_build, build)
+        - if build.active?
+          = link_to cancel_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Cancel' do
+            %i.fa.fa-remove.cred
+        - elsif defined?(allow_retry) && allow_retry && build.retryable?
+          = link_to retry_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Retry' do
+            %i.fa.fa-repeat

app/views/projects/commit/_builds.html.haml

@@ -43,8 +43,8 @@
           %th Coverage
         %th
     - @ci_commit.refs.each do |ref|
-      = render partial: "projects/commit_statuses/commit_status", collection: @ci_commit.statuses.for_ref(ref).latest.ordered,
-               locals: { coverage: @ci_commit.project.build_coverage_enabled?, stage: true, allow_retry: true }
+      - builds = @ci_commit.statuses.for_ref(ref).latest.ordered
+      = render builds, coverage: @ci_commit.project.build_coverage_enabled?, stage: true, allow_retry: true
 
 - if @ci_commit.retried.any?
   .gray-content-block.second-block
@@ -64,5 +64,4 @@
           - if @ci_commit.project.build_coverage_enabled?
             %th Coverage
           %th
-      = render partial: "projects/commit_statuses/commit_status", collection: @ci_commit.retried,
-               locals: { coverage: @ci_commit.project.build_coverage_enabled?, stage: true }
+      = render @ci_commit.retried, coverage: @ci_commit.project.build_coverage_enabled?, stage: true

app/views/projects/commit_statuses/_commit_status.html.haml

-%tr.commit_status
-  %td.status
-    - if can?(current_user, :read_commit_status, commit_status) && commit_status.target_url
-      = link_to commit_status.target_url, class: "ci-status ci-#{commit_status.status}" do
-        = ci_icon_for_status(commit_status.status)
-        = commit_status.status
-    - else
-      = ci_status_with_icon(commit_status.status)
-
-  %td.commit_status-link
-    - if can?(current_user, :read_commit_status, commit_status) && commit_status.target_url
-      = link_to commit_status.target_url do
-        %strong ##{commit_status.id}
-    - else
-      %strong ##{commit_status.id}
-
-    - if commit_status.show_warning?
-      %i.fa.fa-warning.text-warning{data: { toggle: "tooltip" }, title: "This build is stuck, open it to know more"}
-
-  - if defined?(commit_sha) && commit_sha
-    %td
-      = link_to commit_status.short_sha, namespace_project_commit_path(commit_status.project.namespace, commit_status.project, commit_status.sha), class: "monospace"
-      
-  %td
-    - if commit_status.ref
-      = link_to commit_status.ref, namespace_project_commits_path(commit_status.project.namespace, commit_status.project, commit_status.ref)
-    - else
-      .light none
-
-  - if defined?(runner) && runner
-    %td
-      - if commit_status.try(:runner)
-        = runner_link(commit_status.runner)
-      - else
-        .light none
-
-  - if defined?(stage) && stage
-    %td
-      = commit_status.stage
-
-  %td
-    = commit_status.name
-
-    .pull-right
-      - if commit_status.tags.any?
-        - commit_status.tags.each do |tag|
-          %span.label.label-primary
-            = tag
-      - if commit_status.try(:trigger_request)
-        %span.label.label-info triggered
-      - if commit_status.try(:allow_failure)
-        %span.label.label-danger allowed to fail
-
-  %td.duration
-    - if commit_status.duration
-      #{duration_in_words(commit_status.finished_at, commit_status.started_at)}
-
-  %td.timestamp
-    - if commit_status.finished_at
-      %span #{time_ago_with_tooltip(commit_status.finished_at)}
-
-  - if defined?(coverage) && coverage
-    %td.coverage
-      - if commit_status.try(:coverage)
-        #{commit_status.coverage}%
-
-  %td
-    .pull-right
-      - if can?(current_user, :read_commit_status, commit_status) && commit_status.artifacts_download_url
-        = link_to commit_status.artifacts_download_url, title: 'Download artifacts' do
-          %i.fa.fa-download
-      - if can?(current_user, :update_commit_status, commit_status)
-        - if commit_status.active?
-          - if commit_status.cancel_url
-            = link_to commit_status.cancel_url, method: :post, title: 'Cancel' do
-              %i.fa.fa-remove.cred
-        - elsif defined?(allow_retry) && allow_retry && commit_status.retry_url
-          = link_to commit_status.retry_url, method: :post, title: 'Retry' do
-            %i.fa.fa-repeat

app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml

+%tr.generic_commit_status
+  %td.status
+    - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url
+      = link_to generic_commit_status.target_url, class: "ci-status ci-#{generic_commit_status.status}" do
+        = ci_status_with_icon(generic_commit_status.status)
+    - else
+      = ci_status_with_icon(generic_commit_status.status)
+
+  %td.generic_commit_status-link
+    - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url
+      = link_to generic_commit_status.target_url do
+        %strong ##{generic_commit_status.id}
+    - else
+      %strong ##{generic_commit_status.id}
+
+  - if defined?(commit_sha) && commit_sha
+    %td
+      = link_to generic_commit_status.short_sha, namespace_project_commit_path(generic_commit_status.project.namespace, generic_commit_status.project, generic_commit_status.sha), class: "monospace"
+      
+  %td
+    - if generic_commit_status.ref
+      = link_to generic_commit_status.ref, namespace_project_commits_path(generic_commit_status.project.namespace, generic_commit_status.project, generic_commit_status.ref)
+    - else
+      .light none
+
+  - if defined?(runner) && runner
+    %td
+      - if generic_commit_status.try(:runner)
+        = runner_link(generic_commit_status.runner)
+      - else
+        .light none
+
+  - if defined?(stage) && stage
+    %td
+      = generic_commit_status.stage
+
+  %td
+    = generic_commit_status.name
+
+    .pull-right
+      - if generic_commit_status.tags.any?
+        - generic_commit_status.tags.each do |tag|
+          %span.label.label-primary
+            = tag
+
+  %td.duration
+    - if generic_commit_status.duration
+      #{duration_in_words(generic_commit_status.finished_at, generic_commit_status.started_at)}
+
+  %td.timestamp
+    - if generic_commit_status.finished_at
+      %span #{time_ago_with_tooltip(generic_commit_status.finished_at)}
+
+  - if defined?(coverage) && coverage
+    %td.coverage
+      - if generic_commit_status.try(:coverage)
+        #{generic_commit_status.coverage}%
+
+  %td

doc/api/builds.md

@@ -33,7 +33,6 @@ Example of response
         },
         "coverage": null,
         "created_at": "2015-12-24T15:51:21.802Z",
-        "download_url": null,
         "artifacts_file": {
           "filename": "artifacts.zip",
           "size": 1000
@@ -75,7 +74,6 @@ Example of response
         },
         "coverage": null,
         "created_at": "2015-12-24T15:51:21.727Z",
-        "download_url": null,
         "artifacts_file": null,
         "finished_at": "2015-12-24T17:54:24.921Z",
         "id": 6,
@@ -139,7 +137,6 @@ Example of response
         },
         "coverage": null,
         "created_at": "2016-01-11T10:13:33.506Z",
-        "download_url": null,
         "artifacts_file": null,
         "finished_at": "2016-01-11T10:14:09.526Z",
         "id": 69,
@@ -164,7 +161,6 @@ Example of response
         },
         "coverage": null,
         "created_at": "2015-12-24T15:51:21.957Z",
-        "download_url": null,
         "artifacts_file": null,
         "finished_at": "2015-12-24T17:54:33.913Z",
         "id": 9,
@@ -226,7 +222,6 @@ Example of response
     },
     "coverage": null,
     "created_at": "2015-12-24T15:51:21.880Z",
-    "download_url": null,
     "artifacts_file": null,
     "finished_at": "2015-12-24T17:54:31.198Z",
     "id": 8,
@@ -315,7 +310,6 @@ Example of response
     },
     "coverage": null,
     "created_at": "2016-01-11T10:13:33.506Z",
-    "download_url": null,
     "artifacts_file": null,
     "finished_at": "2016-01-11T10:14:09.526Z",
     "id": 69,
@@ -362,7 +356,6 @@ Example of response
     },
     "coverage": null,
     "created_at": "2016-01-11T10:13:33.506Z",
-    "download_url": null,
     "artifacts_file": null,
     "finished_at": null,
     "id": 69,

features/steps/shared/builds.rb

@@ -68,7 +68,7 @@ module SharedBuilds
   end
 
   step 'I see the build' do
-    page.within('.commit_status') do
+    page.within('.build') do
       expect(page).to have_content "##{@build.id}"
       expect(page).to have_content @build.sha[0..7]
       expect(page).to have_content @build.ref

lib/api/entities.rb

@@ -408,13 +408,6 @@ module API
       expose :id, :status, :stage, :name, :ref, :tag, :coverage
       expose :created_at, :started_at, :finished_at
       expose :user, with: User
-      # TODO: download_url in Ci:Build model is an GitLab Web Interface URL, not API URL. We should think on some API
-      #       for downloading of artifacts (see: https://gitlab.com/gitlab-org/gitlab-ce/issues/4255)
-      expose :download_url do |repo_obj, options|
-        if options[:user_can_download_artifacts]
-          repo_obj.artifacts_download_url
-        end
-      end
       expose :artifacts_file, using: BuildArtifactFile, if: -> (build, opts) { build.artifacts? }
       expose :commit, with: RepoCommit do |repo_obj, _options|
         if repo_obj.respond_to?(:commit)

spec/models/build_spec.rb

@@ -9,7 +9,7 @@ describe Ci::Build, models: true do
 
   it { is_expected.to respond_to :trace_html }
 
-  describe :first_pending do
+  describe '#first_pending' do
     let(:first) { FactoryGirl.create :ci_build, commit: commit, status: 'pending', created_at: Date.yesterday }
     let(:second) { FactoryGirl.create :ci_build, commit: commit, status: 'pending' }
     before { first; second }
@@ -19,7 +19,7 @@ describe Ci::Build, models: true do
     it('returns with the first pending build') { is_expected.to eq(first) }
   end
 
-  describe :create_from do
+  describe '#create_from' do
     before do
       build.status = 'success'
       build.save
@@ -33,7 +33,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :ignored? do
+  describe '#ignored?' do
     subject { build.ignored? }
 
     context 'if build is not allowed to fail' do
@@ -69,7 +69,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :trace do
+  describe '#trace' do
     subject { build.trace_html }
 
     it { is_expected.to be_empty }
@@ -101,7 +101,7 @@ describe Ci::Build, models: true do
   #   it { is_expected.to eq(commit.project.timeout) }
   # end
 
-  describe :options do
+  describe '#options' do
     let(:options) do
       {
         image: "ruby:2.1",
@@ -122,25 +122,25 @@ describe Ci::Build, models: true do
   #   it { is_expected.to eq(project.allow_git_fetch) }
   # end
 
-  describe :project do
+  describe '#project' do
     subject { build.project }
 
     it { is_expected.to eq(commit.project) }
   end
 
-  describe :project_id do
+  describe '#project_id' do
     subject { build.project_id }
 
     it { is_expected.to eq(commit.project_id) }
   end
 
-  describe :project_name do
+  describe '#project_name' do
     subject { build.project_name }
 
     it { is_expected.to eq(project.name) }
   end
 
-  describe :extract_coverage do
+  describe '#extract_coverage' do
     context 'valid content & regex' do
       subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', '\(\d+.\d+\%\) covered') }
 
@@ -172,7 +172,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :variables do
+  describe '#variables' do
     context 'returns variables' do
       subject { build.variables }
 
@@ -242,7 +242,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :can_be_served? do
+  describe '#can_be_served?' do
     let(:runner) { FactoryGirl.create :ci_runner }
 
     before { build.project.runners << runner }
@@ -277,7 +277,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :any_runners_online? do
+  describe '#any_runners_online?' do
     subject { build.any_runners_online? }
 
     context 'when no runners' do
@@ -312,8 +312,8 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :show_warning? do
-    subject { build.show_warning? }
+  describe '#stuck?' do
+    subject { build.stuck? }
 
     %w(pending).each do |state|
       context "if commit_status.status is #{state}" do
@@ -343,35 +343,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :artifacts_download_url do
-    subject { build.artifacts_download_url }
-
-    context 'artifacts file does not exist' do
-      before { build.update_attributes(artifacts_file: nil) }
-      it { is_expected.to be_nil }
-    end
-
-    context 'artifacts file exists' do
-      let(:build) { create(:ci_build, :artifacts) }
-      it { is_expected.to_not be_nil }
-    end
-  end
-
-  describe :artifacts_browse_url do
-    subject { build.artifacts_browse_url }
-
-    it "should be nil if artifacts browser is unsupported" do
-      allow(build).to receive(:artifacts_metadata?).and_return(false)
-      is_expected.to be_nil
-    end
-
-    it 'should not be nil if artifacts browser is supported' do
-      allow(build).to receive(:artifacts_metadata?).and_return(true)
-      is_expected.to_not be_nil
-    end
-  end
-
-  describe :artifacts? do
+  describe '#artifacts?' do
     subject { build.artifacts? }
 
     context 'artifacts archive does not exist' do
@@ -386,7 +358,7 @@ describe Ci::Build, models: true do
   end
 
 
-  describe :artifacts_metadata? do
+  describe '#artifacts_metadata?' do
     subject { build.artifacts_metadata? }
     context 'artifacts metadata does not exist' do
       it { is_expected.to be_falsy }
@@ -398,7 +370,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :repo_url do
+  describe '#repo_url' do
     let(:build) { FactoryGirl.create :ci_build }
     let(:project) { build.project }
 
@@ -412,7 +384,7 @@ describe Ci::Build, models: true do
     it { is_expected.to include(project.web_url[7..-1]) }
   end
 
-  describe :depends_on_builds do
+  describe '#depends_on_builds' do
     let!(:build) { FactoryGirl.create :ci_build, commit: commit, name: 'build', stage_idx: 0, stage: 'build' }
     let!(:rspec_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rspec', stage_idx: 1, stage: 'test' }
     let!(:rubocop_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rubocop', stage_idx: 1, stage: 'test' }
@@ -444,7 +416,7 @@ describe Ci::Build, models: true do
                        created_at: created_at)
   end
 
-  describe :merge_request do
+  describe '#merge_request' do
     context 'when a MR has a reference to the commit' do
       before do
         @merge_request = create_mr(build, commit, factory: :merge_request)

spec/models/ci/commit_spec.rb

@@ -32,50 +32,6 @@ describe Ci::Commit, models: true do
   it { is_expected.to respond_to :git_author_email }
   it { is_expected.to respond_to :short_sha }
 
-  describe :ordered do
-    let(:project) { FactoryGirl.create :empty_project }
-
-    it 'returns ordered list of commits' do
-      commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project
-      commit2 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project
-      expect(project.ci_commits.ordered).to eq([commit2, commit1])
-    end
-
-    it 'returns commits ordered by committed_at and id, with nulls last' do
-      commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project
-      commit2 = FactoryGirl.create :ci_commit, committed_at: nil, project: project
-      commit3 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project
-      commit4 = FactoryGirl.create :ci_commit, committed_at: nil, project: project
-      expect(project.ci_commits.ordered).to eq([commit2, commit4, commit3, commit1])
-    end
-  end
-
-  describe :last_build do
-    subject { commit.last_build }
-    before do
-      @first = FactoryGirl.create :ci_build, commit: commit, created_at: Date.yesterday
-      @second = FactoryGirl.create :ci_build, commit: commit
-    end
-
-    it { is_expected.to be_a(Ci::Build) }
-    it('returns with the most recently created build') { is_expected.to eq(@second) }
-  end
-
-  describe :retry do
-    before do
-      @first = FactoryGirl.create :ci_build, commit: commit, created_at: Date.yesterday
-      @second = FactoryGirl.create :ci_build, commit: commit
-    end
-
-    it "creates only a new build" do
-      expect(commit.builds.count(:all)).to eq 2
-      expect(commit.statuses.count(:all)).to eq 2
-      commit.retry
-      expect(commit.builds.count(:all)).to eq 3
-      expect(commit.statuses.count(:all)).to eq 3
-    end
-  end
-
   describe :valid_commit_sha do
     context 'commit.sha can not start with 00000000' do
       before do