From 845d29a27f674a64596c0cfd1cd6a2e976a8656d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 19 Aug 2016 15:22:54 +0000 Subject: 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 --- CHANGELOG | 1 + doc/ci/pipelines.md | 2 + lib/gitlab/badge/coverage/report.rb | 3 +- spec/features/projects/badges/coverage_spec.rb | 41 ++++++++++------ spec/lib/gitlab/badge/coverage/report_spec.rb | 67 +++++++++++++++----------- 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 -- cgit v1.2.1