Commit 436fc4f9 authored by Erick Bajao's avatar Erick Bajao

Refactor to use generic daily report results

Implemented a generic table that can cater to other
types of daily CI metrics in the future.
parent 1574963c
# frozen_string_literal: true
module Ci
class DailyCodeCoverage < ApplicationRecord
extend Gitlab::Ci::Model
def self.create_or_update_for_build(build)
ref = connection.quote(build.ref)
name = connection.quote(build.name)
date = connection.quote(build.created_at.to_date)
connection.execute <<-EOF.strip_heredoc
INSERT INTO #{table_name} (project_id, ref, name, date, last_build_id, coverage)
VALUES (#{build.project_id}, #{ref}, #{name}, #{date}, #{build.id}, #{build.coverage})
ON CONFLICT (project_id, ref, name, date)
DO UPDATE SET coverage = #{build.coverage}, last_build_id = #{build.id} WHERE #{table_name}.last_build_id < #{build.id};
EOF
end
end
end
# frozen_string_literal: true
module Ci
class DailyReportResult < ApplicationRecord
extend Gitlab::Ci::Model
belongs_to :last_pipeline, class_name: 'Ci::Pipeline', foreign_key: :last_pipeline_id
belongs_to :project
# TODO: Refactor this out when BuildReportResult is implemented.
# They both need to share the same enum values for param.
REPORT_PARAMS = {
coverage: 12
}.freeze
enum param: REPORT_PARAMS
def self.store_coverage(pipeline)
return unless Feature.enabled?(:ci_daily_code_coverage, default_enabled: true)
ref_path = connection.quote(pipeline.source_ref_path)
date = connection.quote(pipeline.created_at.to_date)
param = params[:coverage]
pipeline.builds.with_coverage.each do |build|
title = connection.quote(build.group_name)
connection.execute <<-EOF.strip_heredoc
INSERT INTO #{table_name} (project_id, ref_path, param, title, date, last_pipeline_id, value)
VALUES (#{build.project_id}, #{ref_path}, #{param}, #{title}, #{date}, #{pipeline.id}, #{build.coverage})
ON CONFLICT (project_id, ref_path, param, title, date)
DO UPDATE SET value = #{build.coverage}, last_pipeline_id = #{pipeline.id} WHERE #{table_name}.last_pipeline_id < #{pipeline.id};
EOF
end
end
end
end
......@@ -82,6 +82,8 @@ module Ci
has_one :pipeline_config, class_name: 'Ci::PipelineConfig', inverse_of: :pipeline
has_many :daily_report_results, class_name: 'Ci::DailyReportResult', foreign_key: :last_pipeline_id
accepts_nested_attributes_for :variables, reject_if: :persisted?
delegate :id, to: :project, prefix: true
......@@ -189,10 +191,10 @@ module Ci
end
after_transition [:created, :waiting_for_resource, :preparing, :pending, :running] => :success do |pipeline|
# We want to wait a little bit to ensure that all BuildFinishedWorkers finish first
# because this is where code coverage is parsed and stored in CI build records which
# the daily code coverage worker relies on.
pipeline.run_after_commit { Ci::DailyCodeCoverageWorker.perform_in(5.minutes, pipeline.id) }
# We wait a little bit to ensure that all BuildFinishedWorkers finish first
# because this is where some metrics like code coverage is parsed and stored
# in CI build records which the daily build metrics worker relies on.
pipeline.run_after_commit { Ci::DailyReportResultsWorker.perform_in(10.minutes, pipeline.id) }
end
after_transition do |pipeline, transition|
......@@ -944,6 +946,14 @@ module Ci
Ci::PipelineEnums.ci_config_sources.key?(config_source.to_sym)
end
def source_ref_path
if branch? || merge_request?
Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s
elsif tag?
Gitlab::Git::TAG_REF_PREFIX + ref.to_s
end
end
private
def pipeline_data
......
......@@ -314,6 +314,8 @@ class Project < ApplicationRecord
has_many :import_failures, inverse_of: :project
has_many :daily_report_results, class_name: 'Ci::DailyReportResult'
accepts_nested_attributes_for :variables, allow_destroy: true
accepts_nested_attributes_for :project_feature, update_only: true
accepts_nested_attributes_for :import_data
......
# frozen_string_literal: true
module Ci
class DailyCodeCoverageService
def execute(pipeline)
return unless Feature.enabled?(:ci_daily_code_coverage, default_enabled: true)
pipeline.builds.with_coverage.each do |build|
DailyCodeCoverage.create_or_update_for_build(build)
end
end
end
end
# frozen_string_literal: true
module Ci
class DailyReportResultService
def execute(pipeline)
DailyReportResult.store_coverage(pipeline)
end
end
end
......@@ -605,7 +605,7 @@
:resource_boundary: :unknown
:weight: 1
:idempotent:
- :name: pipeline_background:ci_daily_code_coverage
- :name: pipeline_background:ci_daily_report_results
:feature_category: :continuous_integration
:has_external_dependencies:
:urgency: :default
......@@ -752,13 +752,6 @@
:resource_boundary: :unknown
:weight: 5
:idempotent:
- :name: pipeline_processing:pipeline_success
:feature_category: :continuous_integration
:has_external_dependencies:
:urgency: :high
:resource_boundary: :unknown
:weight: 5
:idempotent:
- :name: pipeline_processing:pipeline_update
:feature_category: :continuous_integration
:has_external_dependencies:
......
# frozen_string_literal: true
module Ci
class DailyCodeCoverageWorker
class DailyReportResultsWorker
include ApplicationWorker
include PipelineBackgroundQueue
......@@ -9,7 +9,7 @@ module Ci
def perform(pipeline_id)
Ci::Pipeline.find_by_id(pipeline_id).try do |pipeline|
Ci::DailyCodeCoverageService.new.execute(pipeline)
Ci::DailyReportResultService.new.execute(pipeline)
end
end
end
......
# frozen_string_literal: true
class PipelineSuccessWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
include PipelineQueue
queue_namespace :pipeline_processing
urgency :high
# rubocop: disable CodeReuse/ActiveRecord
def perform(pipeline_id)
Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline|
DailyCodeCoverageWorker.perform_async(pipeline.id)
end
end
# rubocop: enable CodeReuse/ActiveRecord
end
---
title: Store daily code coverages into daily_code_coverages table
title: Store daily code coverages into ci_daily_report_results table
merge_request: 24695
author:
type: added
# frozen_string_literal: true
class CreateDailyCodeCoverages < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
create_table :ci_daily_code_coverages do |t|
t.date :date, null: false
t.bigint :project_id, null: false
t.bigint :last_build_id, null: false
t.float :coverage, null: false
t.string :ref, null: false # rubocop:disable Migration/AddLimitToStringColumns
t.string :name, null: false # rubocop:disable Migration/AddLimitToStringColumns
t.index :last_build_id
t.index [:project_id, :ref, :name, :date], name: 'index_daily_code_coverage_unique_columns', unique: true, order: { date: :desc }
t.foreign_key :projects, on_delete: :cascade
t.foreign_key :ci_builds, column: :last_build_id, on_delete: :cascade
end
end
end
# frozen_string_literal: true
class CreateDailyReportResults < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
create_table :ci_daily_report_results do |t|
t.date :date, null: false
t.bigint :project_id, null: false
t.bigint :last_pipeline_id, null: false
t.float :value, null: false
t.integer :param, limit: 2, null: false
t.string :ref_path, null: false # rubocop:disable Migration/AddLimitToStringColumns
t.string :title, null: false # rubocop:disable Migration/AddLimitToStringColumns
t.index :last_pipeline_id
t.index [:project_id, :ref_path, :param, :title, :date], name: 'index_daily_build_report_metrics_unique_columns', unique: true, order: { date: :desc }
t.foreign_key :projects, on_delete: :cascade
t.foreign_key :ci_pipelines, column: :last_pipeline_id, on_delete: :cascade
end
end
end
......@@ -738,15 +738,16 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do
t.index ["build_id"], name: "index_ci_builds_runner_session_on_build_id", unique: true
end
create_table "ci_daily_code_coverages", force: :cascade do |t|
create_table "ci_daily_report_results", force: :cascade do |t|
t.date "date", null: false
t.bigint "project_id", null: false
t.bigint "last_build_id", null: false
t.float "coverage", null: false
t.string "ref", null: false
t.string "name", null: false
t.index ["last_build_id"], name: "index_ci_daily_code_coverages_on_last_build_id"
t.index ["project_id", "ref", "name", "date"], name: "index_daily_code_coverage_unique_columns", unique: true, order: { date: :desc }
t.bigint "last_pipeline_id", null: false
t.float "value", null: false
t.integer "param", limit: 2, null: false
t.string "ref_path", null: false
t.string "title", null: false
t.index ["last_pipeline_id"], name: "index_ci_daily_report_results_on_last_pipeline_id"
t.index ["project_id", "ref_path", "param", "title", "date"], name: "index_daily_build_report_metrics_unique_columns", unique: true, order: { date: :desc }
end
create_table "ci_group_variables", id: :serial, force: :cascade do |t|
......@@ -4776,8 +4777,8 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do
add_foreign_key "ci_builds_metadata", "ci_builds", column: "build_id", on_delete: :cascade
add_foreign_key "ci_builds_metadata", "projects", on_delete: :cascade
add_foreign_key "ci_builds_runner_session", "ci_builds", column: "build_id", on_delete: :cascade
add_foreign_key "ci_daily_code_coverages", "ci_builds", column: "last_build_id", on_delete: :cascade
add_foreign_key "ci_daily_code_coverages", "projects", on_delete: :cascade
add_foreign_key "ci_daily_report_results", "ci_pipelines", column: "last_pipeline_id", on_delete: :cascade
add_foreign_key "ci_daily_report_results", "projects", on_delete: :cascade
add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade
add_foreign_key "ci_job_artifacts", "ci_builds", column: "job_id", on_delete: :cascade
add_foreign_key "ci_job_artifacts", "projects", on_delete: :cascade
......
# frozen_string_literal: true
FactoryBot.define do
factory :ci_daily_code_coverage, class: 'Ci::DailyCodeCoverage' do
ref { 'test_branch' }
name { 'test_coverage_job' }
coverage { 77 }
date { Time.zone.now.to_date }
after(:build) do |dcc|
build = create(:ci_build)
dcc.project_id = build.project_id unless dcc.project_id
dcc.last_build_id = build.id unless dcc.last_build_id
end
end
end
# frozen_string_literal: true
FactoryBot.define do
factory :ci_daily_report_result, class: 'Ci::DailyReportResult' do
ref_path { Gitlab::Git::BRANCH_REF_PREFIX + 'master' }
date { Time.zone.now.to_date }
project
last_pipeline factory: :ci_pipeline
param { Ci::DailyReportResult.params[:coverage] }
title { 'rspec' }
value { 77.0 }
end
end
......@@ -208,6 +208,7 @@ ci_pipelines:
- vulnerability_findings
- pipeline_config
- security_scans
- daily_report_results
pipeline_variables:
- pipeline
stages:
......@@ -470,6 +471,7 @@ project:
- status_page_setting
- requirements
- export_jobs
- daily_report_results
award_emoji:
- awardable
- user
......
# frozen_string_literal: true
require 'spec_helper'
describe Ci::DailyCodeCoverage do
describe '::create_or_update_for_build' do
let!(:build) { create(:ci_build, created_at: '2020-02-06 00:01:10', name: 'rspec', coverage: 80) }
context 'when there is no existing record with matching project_id, ref, name, date' do
it 'creates a new record for the given build' do
described_class.create_or_update_for_build(build)
expect(described_class.last).to have_attributes(
project_id: build.project.id,
last_build_id: build.id,
ref: build.ref,
name: build.name,
coverage: build.coverage,
date: build.created_at.to_date
)
end
end
context 'when there is existing record with matching project_id, ref, name, date' do
let!(:new_build) { create(:ci_build, project: build.project, created_at: build.created_at, ref: build.ref, name: build.name, coverage: 99) }
let!(:existing) do
create(
:ci_daily_code_coverage,
project_id: existing_build.project.id,
last_build_id: existing_build.id,
ref: existing_build.ref,
name: existing_build.name,
coverage: existing_build.coverage,
date: existing_build.created_at.to_date
)
end
context 'and build ID is newer than last_build_id' do
let(:existing_build) { build }
it 'updates the last_build_id and coverage' do
described_class.create_or_update_for_build(new_build)
existing.reload
expect(existing).to have_attributes(
last_build_id: new_build.id,
coverage: new_build.coverage
)
end
end
context 'and build ID is not newer than last_build_id' do
let(:existing_build) { new_build }
it 'does not update the last_build_id and coverage' do
described_class.create_or_update_for_build(build)
existing.reload
expect(existing).to have_attributes(
last_build_id: new_build.id,
coverage: new_build.coverage
)
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Ci::DailyReportResult do
describe '::store_coverage' do
let!(:pipeline) { create(:ci_pipeline, created_at: '2020-02-06 00:01:10') }
let!(:rspec_job) { create(:ci_build, pipeline: pipeline, name: '3/3 rspec', coverage: 80) }
let!(:karma_job) { create(:ci_build, pipeline: pipeline, name: '2/2 karma', coverage: 90) }
let!(:extra_job) { create(:ci_build, pipeline: pipeline, name: 'extra', coverage: nil) }
it 'creates daily code coverage record for each job in the pipeline that has coverage value' do
described_class.store_coverage(pipeline)
Ci::DailyReportResult.find_by(title: 'rspec').tap do |coverage|
expect(coverage).to have_attributes(
project_id: pipeline.project.id,
last_pipeline_id: pipeline.id,
ref_path: pipeline.source_ref_path,
param: 'coverage',
title: rspec_job.group_name,
value: rspec_job.coverage,
date: pipeline.created_at.to_date
)
end
Ci::DailyReportResult.find_by(title: 'karma').tap do |coverage|
expect(coverage).to have_attributes(
project_id: pipeline.project.id,
last_pipeline_id: pipeline.id,
ref_path: pipeline.source_ref_path,
param: 'coverage',
title: karma_job.group_name,
value: karma_job.coverage,
date: pipeline.created_at.to_date
)
end
expect(Ci::DailyReportResult.find_by(title: 'extra')).to be_nil
end
context 'when there is an existing daily code coverage for the matching date, project, ref_path, and group name' do
let!(:new_pipeline) do
create(
:ci_pipeline,
project: pipeline.project,
ref: pipeline.ref,
created_at: '2020-02-06 00:02:20'
)
end
let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, name: '4/4 rspec', coverage: 84) }
let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, name: '3/3 karma', coverage: 92) }
before do
# Create the existing daily code coverage records
described_class.store_coverage(pipeline)
end
it "updates the existing record's coverage value and last_pipeline_id" do
rspec_coverage = Ci::DailyReportResult.find_by(title: 'rspec')
karma_coverage = Ci::DailyReportResult.find_by(title: 'karma')
# Bump up the coverage values
described_class.store_coverage(new_pipeline)
rspec_coverage.reload
karma_coverage.reload
expect(rspec_coverage).to have_attributes(
last_pipeline_id: new_pipeline.id,
value: new_rspec_job.coverage
)
expect(karma_coverage).to have_attributes(
last_pipeline_id: new_pipeline.id,
value: new_karma_job.coverage
)
end
end
context 'when the ID of the pipeline is older than the last_pipeline_id' do
let!(:new_pipeline) do
create(
:ci_pipeline,
project: pipeline.project,
ref: pipeline.ref,
created_at: '2020-02-06 00:02:20'
)
end
let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, name: '4/4 rspec', coverage: 84) }
let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, name: '3/3 karma', coverage: 92) }
before do
# Create the existing daily code coverage records
# but in this case, for the newer pipeline first.
described_class.store_coverage(new_pipeline)
end
it 'does not update the existing daily code coverage records' do
rspec_coverage = Ci::DailyReportResult.find_by(title: 'rspec')
karma_coverage = Ci::DailyReportResult.find_by(title: 'karma')
# Run another one but for the older pipeline.
# This simulates the scenario wherein the success worker
# of an older pipeline, for some network hiccup, was delayed
# and only got executed right after the newer pipeline's success worker.
# In this case, we don't want to bump the coverage value with an older one.
described_class.store_coverage(pipeline)
rspec_coverage.reload
karma_coverage.reload
expect(rspec_coverage).to have_attributes(
last_pipeline_id: new_pipeline.id,
value: new_rspec_job.coverage
)
expect(karma_coverage).to have_attributes(
last_pipeline_id: new_pipeline.id,
value: new_karma_job.coverage
)
end
end
end
end
......@@ -1120,7 +1120,7 @@ describe Ci::Pipeline, :mailer do
let(:from_status) { status }
it 'schedules pipeline success worker' do
expect(Ci::DailyCodeCoverageWorker).to receive(:perform_in).with(5.minutes, pipeline.id)
expect(Ci::DailyReportResultsWorker).to receive(:perform_in).with(10.minutes, pipeline.id)
pipeline.succeed
end
......@@ -3114,4 +3114,25 @@ describe Ci::Pipeline, :mailer do
end
end
end
describe '#source_ref_path' do
subject { pipeline.source_ref_path }
context 'when pipeline is for a branch' do
it { is_expected.to eq(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.ref.to_s) }
end
context 'when pipeline is for a merge request' do
let(:merge_request) { create(:merge_request, source_project: project) }
let(:pipeline) { create(:ci_pipeline, project: project, head_pipeline_of: merge_request) }
it { is_expected.to eq(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.ref.to_s) }
end
context 'when pipeline is for a tag' do
let(:pipeline) { create(:ci_pipeline, project: project, tag: true) }
it { is_expected.to eq(Gitlab::Git::TAG_REF_PREFIX + pipeline.ref.to_s) }
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Ci::DailyCodeCoverageService, '#execute' do
let!(:pipeline) { create(:ci_pipeline) }
let!(:rspec_job) { create(:ci_build, pipeline: pipeline, created_at: '2020-02-06 00:01:10', name: 'rspec', coverage: 80) }
let!(:karma_job) { create(:ci_build, pipeline: pipeline, created_at: '2020-02-06 00:01:12', name: 'karma', coverage: 90) }
let!(:extra_job) { create(:ci_build, pipeline: pipeline, created_at: '2020-02-06 00:01:14', name: 'extra', coverage: nil) }
it 'creates daily code coverage record for each job in the pipeline that has coverage value' do
described_class.new.execute(pipeline)
Ci::DailyCodeCoverage.find_by(name: 'rspec').tap do |coverage|
expect(coverage).to have_attributes(
project_id: pipeline.project.id,
last_build_id: rspec_job.id,
ref: pipeline.ref,
name: rspec_job.name,
coverage: rspec_job.coverage,
date: rspec_job.created_at.to_date
)
end
Ci::DailyCodeCoverage.find_by(name: 'karma').tap do |coverage|
expect(coverage).to have_attributes(
project_id: pipeline.project.id,
last_build_id: karma_job.id,
ref: pipeline.ref,
name: karma_job.name,
coverage: karma_job.coverage,
date: karma_job.created_at.to_date
)
end
expect(Ci::DailyCodeCoverage.find_by(name: 'extra')).to be_nil
end
context 'when there is an existing daily code coverage for the matching date, project, ref, and name' do
let!(:new_pipeline) do
create(
:ci_pipeline,
project: pipeline.project,
ref: pipeline.ref
)
end
let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, created_at: '2020-02-06 00:02:20', name: 'rspec', coverage: 84) }
let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, created_at: '2020-02-06 00:02:22', name: 'karma', coverage: 92) }
before do
# Create the existing daily code coverage records
described_class.new.execute(pipeline)
end
it "updates the existing record's coverage value and last_build_id" do
rspec_coverage = Ci::DailyCodeCoverage.find_by(name: 'rspec')
karma_coverage = Ci::DailyCodeCoverage.find_by(name: 'karma')
# Bump up the coverage values
described_class.new.execute(new_pipeline)
rspec_coverage.reload
karma_coverage.reload
expect(rspec_coverage).to have_attributes(
last_build_id: new_rspec_job.id,
coverage: new_rspec_job.coverage
)
expect(karma_coverage).to have_attributes(
last_build_id: new_karma_job.id,
coverage: new_karma_job.coverage
)
end
end
context 'when the ID of the build is older than the last_build_id' do
let!(:new_pipeline) do
create(
:ci_pipeline,
project: pipeline.project,
ref: pipeline.ref
)
end
let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, created_at: '2020-02-06 00:02:20', name: 'rspec', coverage: 84) }
let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, created_at: '2020-02-06 00:02:22', name: 'karma', coverage: 92) }
before do
# Create the existing daily code coverage records
# but in this case, for the newer pipeline first.
described_class.new.execute(new_pipeline)
end
it 'does not update the existing daily code coverage records' do
rspec_coverage = Ci::DailyCodeCoverage.find_by(name: 'rspec')
karma_coverage = Ci::DailyCodeCoverage.find_by(name: 'karma')
# Run another one but for the older pipeline.
# This simulates the scenario wherein the success worker
# of an older pipeline, for some network hiccup, was delayed
# and only got executed right after the newer pipeline's success worker.
# In this case, we don't want to bump the coverage value with an older one.
described_class.new.execute(pipeline)
rspec_coverage.reload
karma_coverage.reload
expect(rspec_coverage).to have_attributes(
last_build_id: new_rspec_job.id,
coverage: new_rspec_job.coverage
)
expect(karma_coverage).to have_attributes(
last_build_id: new_karma_job.id,
coverage: new_karma_job.coverage
)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Ci::DailyReportResultService, '#execute' do
let(:pipeline) { double }
it 'stores daily code coverage' do
expect(Ci::DailyReportResult).to receive(:store_coverage).with(pipeline)
described_class.new.execute(pipeline)
end
end
......@@ -2,7 +2,7 @@
require 'spec_helper'
describe Ci::DailyCodeCoverageWorker do
describe Ci::DailyReportResultsWorker do
describe '#perform' do
let!(:pipeline) { create(:ci_pipeline) }
......@@ -12,7 +12,7 @@ describe Ci::DailyCodeCoverageWorker do
let(:pipeline_id) { pipeline.id }
it 'executes service' do
expect_any_instance_of(Ci::DailyCodeCoverageService)
expect_any_instance_of(Ci::DailyReportResultService)
.to receive(:execute).with(pipeline)
subject
......@@ -23,7 +23,7 @@ describe Ci::DailyCodeCoverageWorker do
let(:pipeline_id) { 123 }
it 'does not execute service' do
expect_any_instance_of(Ci::DailyCodeCoverageService)
expect_any_instance_of(Ci::DailyReportResultService)
.not_to receive(:execute)
expect { subject }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment