Merge branch 'zj-pipeline-badge-improvements' into 'master'

Pipeline badge improvements

Closes #15582 and #20961

See merge request !12966

15 files changed, 159 insertions, 114 deletions

diff --git a/app/controllers/projects/badges_controller.rb b/app/controllers/projects/badges_controller.rb
index 6c25cd83a24..06ba73d8e8d 100644
--- a/app/controllers/projects/badges_controller.rb
+++ b/app/controllers/projects/badges_controller.rb
@@ -3,11 +3,11 @@ class Projects::BadgesController < Projects::ApplicationController
   before_action :authorize_admin_project!, only: [:index]
   before_action :no_cache_headers, except: [:index]
 
-  def build
-    build_status = Gitlab::Badge::Build::Status
+  def pipeline
+    pipeline_status = Gitlab::Badge::Pipeline::Status
       .new(project, params[:ref])
 
-    render_badge build_status
+    render_badge pipeline_status
   end
 
   def coverage
diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb
index ea7ceb3eaa5..15a2ff56b92 100644
--- a/app/controllers/projects/settings/ci_cd_controller.rb
+++ b/app/controllers/projects/settings/ci_cd_controller.rb
@@ -35,7 +35,7 @@ module Projects
       def define_badges_variables
         @ref = params[:ref] || @project.default_branch || 'master'
 
-        @badges = [Gitlab::Badge::Build::Status,
+        @badges = [Gitlab::Badge::Pipeline::Status,
                    Gitlab::Badge::Coverage::Report]
 
         @badges.map! do |badge|
diff --git a/changelogs/unreleased/zj-pipeline-badge-improvements.yml b/changelogs/unreleased/zj-pipeline-badge-improvements.yml
new file mode 100644
index 00000000000..735192ede2d
--- /dev/null
+++ b/changelogs/unreleased/zj-pipeline-badge-improvements.yml
@@ -0,0 +1,4 @@
+---
+title: Update build badges to be pipeline badges and display passing instead of success
+merge_request:
+author:
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 672b5a9a160..06928c7b9ce 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -379,7 +379,9 @@ constraints(ProjectUrlConstrainer.new) do
         collection do
           scope '*ref', constraints: { ref: Gitlab::PathRegex.git_reference_regex } do
             constraints format: /svg/ do
-              get :build
+              # Keep around until 10.0, see gitlab-org/gitlab-ce#35307
+              get :build, to: "badges#pipeline"
+              get :pipeline
               get :coverage
             end
           end
diff --git a/features/project/badges/build.feature b/features/project/badges/build.feature
deleted file mode 100644
index bcf80ed620e..00000000000
--- a/features/project/badges/build.feature
+++ /dev/null
@@ -1,27 +0,0 @@
-Feature: Project Badges Build
-  Background:
-    Given I sign in as a user
-    And I own a project
-    And project has CI enabled
-    And project has a recent build
-
-  Scenario: I want to see a badge for successfully built project
-    Given recent build is successful
-    When I display builds badge for a master branch
-    Then I should see a build success badge
-
-  Scenario: I want to see a badge for project with failed builds
-    Given recent build failed
-    When I display builds badge for a master branch
-    Then I should see a build failed badge
-
-  Scenario: I want to see a badge for project with running builds
-    Given recent build is successful
-    And project has another build that is running
-    When I display builds badge for a master branch
-    Then I should see a build running badge
-
-  Scenario: I want to see a fresh badge on each request
-    Given recent build is successful
-    When I display builds badge for a master branch
-    Then I should see a badge that has not been cached
diff --git a/features/steps/project/badges/build.rb b/features/steps/project/badges/build.rb
deleted file mode 100644
index 5a9094ee9d3..00000000000
--- a/features/steps/project/badges/build.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-class Spinach::Features::ProjectBadgesBuild < Spinach::FeatureSteps
-  include SharedAuthentication
-  include SharedProject
-  include SharedBuilds
-  include RepoHelpers
-
-  step 'I display builds badge for a master branch' do
-    visit build_project_badges_path(@project, ref: :master, format: :svg)
-  end
-
-  step 'I should see a build success badge' do
-    expect_badge('success')
-  end
-
-  step 'I should see a build failed badge' do
-    expect_badge('failed')
-  end
-
-  step 'I should see a build running badge' do
-    expect_badge('running')
-  end
-
-  step 'I should see a badge that has not been cached' do
-    expect(page.response_headers['Cache-Control']).to include 'no-cache'
-  end
-
-  def expect_badge(status)
-    svg = Nokogiri::XML.parse(page.body)
-    expect(page.response_headers['Content-Type']).to include('image/svg+xml')
-    expect(svg.at(%Q{text:contains("#{status}")})).to be_truthy
-  end
-end
diff --git a/lib/gitlab/badge/build/metadata.rb b/lib/gitlab/badge/pipeline/metadata.rb
index 2ee35a0d4c1..db1e9f8cfb8 100644
--- a/lib/gitlab/badge/build/metadata.rb
+++ b/lib/gitlab/badge/pipeline/metadata.rb
@@ -1,8 +1,8 @@
 module Gitlab
   module Badge
-    module Build
+    module Pipeline
       ##
-      # Class that describes build badge metadata
+      # Class that describes pipeline badge metadata
       #
       class Metadata < Badge::Metadata
         def initialize(badge)
@@ -11,11 +11,11 @@ module Gitlab
         end
 
         def title
-          'build status'
+          'pipeline status'
         end
 
         def image_url
-          build_project_badges_url(@project, @ref, format: :svg)
+          pipeline_project_badges_url(@project, @ref, format: :svg)
         end
 
         def link_url
diff --git a/lib/gitlab/badge/build/status.rb b/lib/gitlab/badge/pipeline/status.rb
index b762d85b6e5..5fee7a93475 100644
--- a/lib/gitlab/badge/build/status.rb
+++ b/lib/gitlab/badge/pipeline/status.rb
@@ -1,8 +1,8 @@
 module Gitlab
   module Badge
-    module Build
+    module Pipeline
       ##
-      # Build status badge
+      # Pipeline status badge
       #
       class Status < Badge::Base
         attr_reader :project, :ref
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def entity
-          'build'
+          'pipeline'
         end
 
         def status
@@ -25,11 +25,11 @@ module Gitlab
         end
 
         def metadata
-          @metadata ||= Build::Metadata.new(self)
+          @metadata ||= Pipeline::Metadata.new(self)
         end
 
         def template
-          @template ||= Build::Template.new(self)
+          @template ||= Pipeline::Template.new(self)
         end
       end
     end
diff --git a/lib/gitlab/badge/build/template.rb b/lib/gitlab/badge/pipeline/template.rb
index bc0e0cd441d..e09db32262d 100644
--- a/lib/gitlab/badge/build/template.rb
+++ b/lib/gitlab/badge/pipeline/template.rb
@@ -1,12 +1,13 @@
 module Gitlab
   module Badge
-    module Build
+    module Pipeline
       ##
-      # Class that represents a build badge template.
+      # Class that represents a pipeline badge template.
       #
       # Template object will be passed to badge.svg.erb template.
       #
       class Template < Badge::Template
+        STATUS_RENAME = { 'success' => 'passed' }.freeze
         STATUS_COLOR = {
           success: '#4c1',
           failed: '#e05d44',
@@ -27,11 +28,11 @@ module Gitlab
         end
 
         def value_text
-          @status.to_s
+          STATUS_RENAME[@status.to_s] || @status.to_s
         end
 
         def key_width
-          38
+          62
         end
 
         def value_width
diff --git a/spec/controllers/projects/badges_controller_spec.rb b/spec/controllers/projects/badges_controller_spec.rb
new file mode 100644
index 00000000000..d68200164e4
--- /dev/null
+++ b/spec/controllers/projects/badges_controller_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe Projects::BadgesController do
+  let(:project) { pipeline.project }
+  let!(:pipeline) { create(:ci_empty_pipeline) }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_master(user)
+    sign_in(user)
+  end
+
+  it 'requests the pipeline badge successfully' do
+    get_badge(:pipeline)
+
+    expect(response).to have_http_status(:ok)
+  end
+
+  it 'requests the coverage badge successfully' do
+    get_badge(:coverage)
+
+    expect(response).to have_http_status(:ok)
+  end
+
+  def get_badge(badge)
+    get badge, namespace_id: project.namespace.to_param, project_id: project, ref: pipeline.ref, format: :svg
+  end
+end
diff --git a/spec/features/projects/badges/list_spec.rb b/spec/features/projects/badges/list_spec.rb
index 161d731f524..fd8e9232b02 100644
--- a/spec/features/projects/badges/list_spec.rb
+++ b/spec/features/projects/badges/list_spec.rb
@@ -10,16 +10,16 @@ feature 'list of badges' do
   end
 
   scenario 'user wants to see build status badge' do
-    page.within('.build-status') do
-      expect(page).to have_content 'build status'
+    page.within('.pipeline-status') do
+      expect(page).to have_content 'pipeline status'
       expect(page).to have_content 'Markdown'
       expect(page).to have_content 'HTML'
       expect(page).to have_content 'AsciiDoc'
       expect(page).to have_css('.highlight', count: 3)
-      expect(page).to have_xpath("//img[@alt='build status']")
+      expect(page).to have_xpath("//img[@alt='pipeline status']")
 
       page.within('.highlight', match: :first) do
-        expect(page).to have_content 'badges/master/build.svg'
+        expect(page).to have_content 'badges/master/pipeline.svg'
       end
     end
   end
@@ -40,14 +40,14 @@ feature 'list of badges' do
   end
 
   scenario 'user changes current ref of build status badge', js: true do
-    page.within('.build-status') do
+    page.within('.pipeline-status') do
       first('.js-project-refs-dropdown').click
 
       page.within '.project-refs-form' do
         click_link 'improve/awesome'
       end
 
-      expect(page).to have_content 'badges/improve/awesome/build.svg'
+      expect(page).to have_content 'badges/improve/awesome/pipeline.svg'
     end
   end
 end
diff --git a/spec/features/projects/badges/pipeline_badge_spec.rb b/spec/features/projects/badges/pipeline_badge_spec.rb
new file mode 100644
index 00000000000..b83ea8f4eaa
--- /dev/null
+++ b/spec/features/projects/badges/pipeline_badge_spec.rb
@@ -0,0 +1,70 @@
+require 'spec_helper'
+
+feature 'Pipeline Badge' do
+  set(:project) { create(:project, :repository, :public) }
+  let(:ref) { project.default_branch }
+
+  # this can't be tested in the controller, as it bypasses the rails router
+  # and constructs a route based on the controller being tested
+  # Keep around until 10.0, see gitlab-org/gitlab-ce#35307
+  context 'when the deprecated badge is requested' do
+    it 'displays the badge' do
+      visit build_project_badges_path(project, ref: ref, format: :svg)
+
+      expect(page.status_code).to eq(200)
+    end
+  end
+
+  context 'when the project has a pipeline' do
+    let!(:pipeline) { create(:ci_empty_pipeline, project: project, ref: ref, sha: project.commit(ref).sha) }
+    let!(:job) { create(:ci_build, pipeline: pipeline) }
+
+    context 'when the pipeline was successfull' do
+      it 'displays so on the badge' do
+        job.success
+
+        visit pipeline_project_badges_path(project, ref: ref, format: :svg)
+
+        expect(page.status_code).to eq(200)
+        expect_badge('passed')
+      end
+    end
+
+    context 'when the pipeline failed' do
+      it 'shows displays so on the badge' do
+        job.drop
+
+        visit pipeline_project_badges_path(project, ref: ref, format: :svg)
+
+        expect(page.status_code).to eq(200)
+        expect_badge('failed')
+      end
+    end
+
+    context 'when the pipeline is running' do
+      it 'shows displays so on the badge' do
+        create(:ci_build, pipeline: pipeline, name: 'second build', status_event: 'run')
+
+        visit pipeline_project_badges_path(project, ref: ref, format: :svg)
+
+        expect(page.status_code).to eq(200)
+        expect_badge('running')
+      end
+    end
+
+    context 'when a new pipeline is created' do
+      it 'shows a fresh badge' do
+        visit pipeline_project_badges_path(project, ref: ref, format: :svg)
+
+        expect(page.status_code).to eq(200)
+        expect(page.response_headers['Cache-Control']).to include 'no-cache'
+      end
+    end
+
+    def expect_badge(status)
+      svg = Nokogiri::XML.parse(page.body)
+      expect(page.response_headers['Content-Type']).to include('image/svg+xml')
+      expect(svg.at(%Q{text:contains("#{status}")})).to be_truthy
+    end
+  end
+end
diff --git a/spec/lib/gitlab/badge/build/metadata_spec.rb b/spec/lib/gitlab/badge/pipeline/metadata_spec.rb
index 9df96ea04eb..d537ce8803c 100644
--- a/spec/lib/gitlab/badge/build/metadata_spec.rb
+++ b/spec/lib/gitlab/badge/pipeline/metadata_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 require 'lib/gitlab/badge/shared/metadata'
 
-describe Gitlab::Badge::Build::Metadata do
+describe Gitlab::Badge::Pipeline::Metadata do
   let(:badge) { double(project: create(:empty_project), ref: 'feature') }
   let(:metadata) { described_class.new(badge) }
 
@@ -9,13 +9,13 @@ describe Gitlab::Badge::Build::Metadata do
 
   describe '#title' do
     it 'returns build status title' do
-      expect(metadata.title).to eq 'build status'
+      expect(metadata.title).to eq 'pipeline status'
     end
   end
 
   describe '#image_url' do
     it 'returns valid url' do
-      expect(metadata.image_url).to include 'badges/feature/build.svg'
+      expect(metadata.image_url).to include 'badges/feature/pipeline.svg'
     end
   end
 
diff --git a/spec/lib/gitlab/badge/build/status_spec.rb b/spec/lib/gitlab/badge/pipeline/status_spec.rb
index 6abf4ca46a9..dc835375c66 100644
--- a/spec/lib/gitlab/badge/build/status_spec.rb
+++ b/spec/lib/gitlab/badge/pipeline/status_spec.rb
@@ -1,36 +1,35 @@
 require 'spec_helper'
 
-describe Gitlab::Badge::Build::Status do
+describe Gitlab::Badge::Pipeline::Status do
   let(:project) { create(:project, :repository) }
   let(:sha) { project.commit.sha }
   let(:branch) { 'master' }
   let(:badge) { described_class.new(project, branch) }
 
   describe '#entity' do
-    it 'always says build' do
-      expect(badge.entity).to eq 'build'
+    it 'always says pipeline' do
+      expect(badge.entity).to eq 'pipeline'
     end
   end
 
   describe '#template' do
     it 'returns badge template' do
-      expect(badge.template.key_text).to eq 'build'
+      expect(badge.template.key_text).to eq 'pipeline'
     end
   end
 
   describe '#metadata' do
     it 'returns badge metadata' do
-      expect(badge.metadata.image_url)
-        .to include 'badges/master/build.svg'
+      expect(badge.metadata.image_url).to include 'badges/master/pipeline.svg'
     end
   end
 
-  context 'build exists' do
-    let!(:build) { create_build(project, sha, branch) }
+  context 'pipeline exists' do
+    let!(:pipeline) { create_pipeline(project, sha, branch) }
 
-    context 'build success' do
+    context 'pipeline success' do
       before do
-        build.success!
+        pipeline.success!
       end
 
       describe '#status' do
@@ -40,9 +39,9 @@ describe Gitlab::Badge::Build::Status do
       end
     end
 
-    context 'build failed' do
+    context 'pipeline failed' do
       before do
-        build.drop!
+        pipeline.drop!
       end
 
       describe '#status' do
@@ -54,10 +53,10 @@ describe Gitlab::Badge::Build::Status do
 
     context 'when outdated pipeline for given ref exists' do
       before do
-        build.success!
+        pipeline.success!
 
-        old_build = create_build(project, '11eeffdd', branch)
-        old_build.drop!
+        old_pipeline = create_pipeline(project, '11eeffdd', branch)
+        old_pipeline.drop!
       end
 
       it 'does not take outdated pipeline into account' do
@@ -67,10 +66,10 @@ describe Gitlab::Badge::Build::Status do
 
     context 'when multiple pipelines exist for given sha' do
       before do
-        build.drop!
+        pipeline.drop!
 
-        new_build = create_build(project, sha, branch)
-        new_build.success!
+        new_pipeline = create_pipeline(project, sha, branch)
+        new_pipeline.success!
       end
 
       it 'does not take outdated pipeline into account' do
@@ -87,7 +86,7 @@ describe Gitlab::Badge::Build::Status do
     end
   end
 
-  def create_build(project, sha, branch)
+  def create_pipeline(project, sha, branch)
     pipeline = create(:ci_empty_pipeline,
                       project: project,
                       sha: sha,
diff --git a/spec/lib/gitlab/badge/build/template_spec.rb b/spec/lib/gitlab/badge/pipeline/template_spec.rb
index a7e21fb8bb1..20fa4f879c3 100644
--- a/spec/lib/gitlab/badge/build/template_spec.rb
+++ b/spec/lib/gitlab/badge/pipeline/template_spec.rb
@@ -1,28 +1,28 @@
 require 'spec_helper'
 
-describe Gitlab::Badge::Build::Template do
-  let(:badge) { double(entity: 'build', status: 'success') }
+describe Gitlab::Badge::Pipeline::Template do
+  let(:badge) { double(entity: 'pipeline', status: 'success') }
   let(:template) { described_class.new(badge) }
 
   describe '#key_text' do
-    it 'is always says build' do
-      expect(template.key_text).to eq 'build'
+    it 'is always says pipeline' do
+      expect(template.key_text).to eq 'pipeline'
     end
   end
 
   describe '#value_text' do
     it 'is status value' do
-      expect(template.value_text).to eq 'success'
+      expect(template.value_text).to eq 'passed'
     end
   end
 
   describe 'widths and text anchors' do
     it 'has fixed width and text anchors' do
-      expect(template.width).to eq 92
-      expect(template.key_width).to eq 38
+      expect(template.width).to eq 116
+      expect(template.key_width).to eq 62
       expect(template.value_width).to eq 54
-      expect(template.key_text_anchor).to eq 19
-      expect(template.value_text_anchor).to eq 65
+      expect(template.key_text_anchor).to eq 31
+      expect(template.value_text_anchor).to eq 89
     end
   end