Merge branch 'fix/improve-test-coverage-badge-pipelines' into 'master'

Render coverage badge using latest successful pipeline

## What does this MR do?

This MR make test coverage badge to report value for the latest successful pipeline, instead of the latest one, regardless the status.

Latest pipeline is often running, which makes coverage report inaccurate. Latest pipeline can be also the failed one, which may mean that not all stages got processed, therefore coverage report can be inaccurate as well.

This also improves coverage badge performance because it is not necessary to touch repository to get recent SHA on the branch.

## Why was this MR needed?

See #21013

## What are the relevant issue numbers?

Closes #21013

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5862

5 files changed, 69 insertions, 45 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 3b1307e778b..d22c215490e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.11.0 (unreleased)
+  - Use test coverage value from the latest successful pipeline in badge. !5862
   - Add test coverage report badge. !5708
   - Remove the http_parser.rb dependency by removing the tinder gem. !5758 (tbalthazar)
   - Ability to specify branches for Pivotal Tracker integration (Egor Lynko)
diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md
index d90d7aca4fd..20cd88c8d20 100644
--- a/doc/ci/pipelines.md
+++ b/doc/ci/pipelines.md
@@ -67,6 +67,8 @@ use following Markdown code to embed the est coverage report into `README.md`:
 ![coverage](http://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage)
 ```
 
+The latest successful pipeline will be used to read the test coverage value.
+
 [builds]: #builds
 [jobs]: yaml/README.md#jobs
 [stages]: yaml/README.md#stages
diff --git a/lib/gitlab/badge/coverage/report.rb b/lib/gitlab/badge/coverage/report.rb
index 3d56ea3e47a..95d925dc7f3 100644
--- a/lib/gitlab/badge/coverage/report.rb
+++ b/lib/gitlab/badge/coverage/report.rb
@@ -13,8 +13,7 @@ module Gitlab
           @job = job
 
           @pipeline = @project.pipelines
-            .where(ref: @ref)
-            .where(sha: @project.commit(@ref).try(:sha))
+            .latest_successful_for(@ref)
             .first
         end
 
diff --git a/spec/features/projects/badges/coverage_spec.rb b/spec/features/projects/badges/coverage_spec.rb
index af86d3c338a..5972e7f31c2 100644
--- a/spec/features/projects/badges/coverage_spec.rb
+++ b/spec/features/projects/badges/coverage_spec.rb
@@ -4,12 +4,6 @@ feature 'test coverage badge' do
   given!(:user) { create(:user) }
   given!(:project) { create(:project, :private) }
 
-  given!(:pipeline) do
-    create(:ci_pipeline, project: project,
-                         ref: 'master',
-                         sha: project.commit.id)
-  end
-
   context 'when user has access to view badge' do
     background do
       project.team << [user, :developer]
@@ -17,8 +11,10 @@ feature 'test coverage badge' do
     end
 
     scenario 'user requests coverage badge image for pipeline' do
-      create_job(coverage: 100, name: 'test:1')
-      create_job(coverage: 90, name: 'test:2')
+      create_pipeline do |pipeline|
+        create_build(pipeline, coverage: 100, name: 'test:1')
+        create_build(pipeline, coverage: 90, name: 'test:2')
+      end
 
       show_test_coverage_badge
 
@@ -26,9 +22,11 @@ feature 'test coverage badge' do
     end
 
     scenario 'user requests coverage badge for specific job' do
-      create_job(coverage: 50, name: 'test:1')
-      create_job(coverage: 50, name: 'test:2')
-      create_job(coverage: 85, name: 'coverage')
+      create_pipeline do |pipeline|
+        create_build(pipeline, coverage: 50, name: 'test:1')
+        create_build(pipeline, coverage: 50, name: 'test:2')
+        create_build(pipeline, coverage: 85, name: 'coverage')
+      end
 
       show_test_coverage_badge(job: 'coverage')
 
@@ -36,7 +34,9 @@ feature 'test coverage badge' do
     end
 
     scenario 'user requests coverage badge for pipeline without coverage' do
-      create_job(coverage: nil, name: 'test')
+      create_pipeline do |pipeline|
+        create_build(pipeline, coverage: nil, name: 'test')
+      end
 
       show_test_coverage_badge
 
@@ -54,10 +54,19 @@ feature 'test coverage badge' do
     end
   end
 
-  def create_job(coverage:, name:)
-    create(:ci_build, name: name,
-                      coverage: coverage,
-                      pipeline: pipeline)
+  def create_pipeline
+    opts = { project: project, ref: 'master', sha: project.commit.id }
+
+    create(:ci_pipeline, opts).tap do |pipeline|
+      yield pipeline
+      pipeline.build_updated
+    end
+  end
+
+  def create_build(pipeline, coverage:, name:)
+    opts = { pipeline: pipeline, coverage: coverage, name: name }
+
+    create(:ci_build, :success, opts)
   end
 
   def show_test_coverage_badge(job: nil)
diff --git a/spec/lib/gitlab/badge/coverage/report_spec.rb b/spec/lib/gitlab/badge/coverage/report_spec.rb
index 1ff49602486..ab0cce6e091 100644
--- a/spec/lib/gitlab/badge/coverage/report_spec.rb
+++ b/spec/lib/gitlab/badge/coverage/report_spec.rb
@@ -44,45 +44,49 @@ describe Gitlab::Badge::Coverage::Report do
     end
   end
 
-  context 'pipeline exists' do
-    let!(:pipeline) do
-      create(:ci_pipeline, project: project,
-                           sha: project.commit.id,
-                           ref: 'master')
-    end
+  context 'when latest successful pipeline exists' do
+    before do
+      create_pipeline do |pipeline|
+        create(:ci_build, :success, pipeline: pipeline, name: 'first', coverage: 40)
+        create(:ci_build, :success, pipeline: pipeline, coverage: 60)
+      end
 
-    context 'builds exist' do
-      before do
-        create(:ci_build, name: 'first', pipeline: pipeline, coverage: 40)
-        create(:ci_build, pipeline: pipeline, coverage: 60)
+      create_pipeline do |pipeline|
+        create(:ci_build, :failed, pipeline: pipeline, coverage: 10)
       end
+    end
 
-      context 'particular job specified' do
-        let(:job_name) { 'first' }
+    context 'when particular job specified' do
+      let(:job_name) { 'first' }
 
-        it 'returns coverage for the particular job' do
-          expect(badge.status).to eq 40
-        end
+      it 'returns coverage for the particular job' do
+        expect(badge.status).to eq 40
       end
+    end
 
-      context 'particular job not specified' do
-        let(:job_name) { '' }
+    context 'when particular job not specified' do
+      let(:job_name) { '' }
+
+      it 'returns arithemetic mean for the pipeline' do
+        expect(badge.status).to eq 50
+      end
+    end
+  end
 
-        it 'returns arithemetic mean for the pipeline' do
-          expect(badge.status).to eq 50
-        end
+  context 'when only failed pipeline exists' do
+    before do
+      create_pipeline do |pipeline|
+        create(:ci_build, :failed, pipeline: pipeline, coverage: 10)
       end
     end
 
-    context 'builds do not exist' do
-      it_behaves_like 'unknown coverage report'
+    it_behaves_like 'unknown coverage report'
 
-      context 'particular job specified' do
-        let(:job_name) { 'nonexistent' }
+    context 'particular job specified' do
+      let(:job_name) { 'nonexistent' }
 
-        it 'retruns nil' do
-          expect(badge.status).to be_nil
-        end
+      it 'retruns nil' do
+        expect(badge.status).to be_nil
       end
     end
   end
@@ -90,4 +94,13 @@ describe Gitlab::Badge::Coverage::Report do
   context 'pipeline does not exist' do
     it_behaves_like 'unknown coverage report'
   end
+
+  def create_pipeline
+    opts = { project: project, sha: project.commit.id, ref: 'master' }
+
+    create(:ci_pipeline, opts).tap do |pipeline|
+      yield pipeline
+      pipeline.build_updated
+    end
+  end
 end