diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 5d316906bd3c7b21efd935664d51e8269478d69d..d78894078493aead17a7a7b3eeb42464966343fc 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -1004,6 +1004,10 @@ def self_and_descendants
object_hierarchy(project_condition: :same).base_and_descendants
end
+ def self_and_descendants_complete?
+ self_and_descendants.all?(&:complete?)
+ end
+
# Follow the parent-child relationships and return the top-level parent
def root_ancestor
return self unless child?
diff --git a/app/models/ci/pipeline_artifact.rb b/app/models/ci/pipeline_artifact.rb
index 2284a05bcc935a4bec17084a14e89cd49263d40a..cdc3d69f754318a95f72106438ada929d6a71b1b 100644
--- a/app/models/ci/pipeline_artifact.rb
+++ b/app/models/ci/pipeline_artifact.rb
@@ -51,6 +51,23 @@ def report_exists?(file_type)
def find_by_file_type(file_type)
find_by(file_type: file_type)
end
+
+ def create_or_replace_for_pipeline!(pipeline:, file_type:, file:, size:)
+ transaction do
+ pipeline.pipeline_artifacts.find_by_file_type(file_type)&.destroy!
+
+ pipeline.pipeline_artifacts.create!(
+ file_type: file_type,
+ project_id: pipeline.project_id,
+ size: size,
+ file: file,
+ file_format: REPORT_TYPES[file_type],
+ expire_at: EXPIRATION_DATE.from_now
+ )
+ end
+ rescue ActiveRecord::ActiveRecordError => err
+ Gitlab::ErrorTracking.track_and_raise_exception(err, { pipeline_id: pipeline.id, file_type: file_type })
+ end
end
def present
diff --git a/app/services/ci/pipeline_artifacts/coverage_report_service.rb b/app/services/ci/pipeline_artifacts/coverage_report_service.rb
index b0acb1d5a0b98b65c328b0e7ac493c83a05455a9..c69cb721bb4238e0c2b4aec01730650bc3d7059c 100644
--- a/app/services/ci/pipeline_artifacts/coverage_report_service.rb
+++ b/app/services/ci/pipeline_artifacts/coverage_report_service.rb
@@ -9,17 +9,16 @@ def initialize(pipeline)
end
def execute
- return if pipeline.has_coverage_reports?
+ unless Feature.enabled?(:ci_child_pipeline_coverage_reports, pipeline.project) ||
+ !pipeline.has_coverage_reports?
+ return
+ end
+
return if report.empty?
- pipeline.pipeline_artifacts.create!(
- project_id: pipeline.project_id,
- file_type: :code_coverage,
- file_format: Ci::PipelineArtifact::REPORT_TYPES.fetch(:code_coverage),
- size: carrierwave_file["tempfile"].size,
- file: carrierwave_file,
- expire_at: Ci::PipelineArtifact::EXPIRATION_DATE.from_now
- )
+ Ci::PipelineArtifact.create_or_replace_for_pipeline!(**pipeline_artifact_params).tap do |pipeline_artifact|
+ Gitlab::AppLogger.info(log_params(pipeline_artifact))
+ end
end
private
@@ -32,6 +31,15 @@ def report
end
end
+ def pipeline_artifact_params
+ {
+ pipeline: pipeline,
+ file_type: :code_coverage,
+ file: carrierwave_file,
+ size: carrierwave_file['tempfile'].size
+ }
+ end
+
def carrierwave_file
strong_memoize(:carrier_wave_file) do
CarrierWaveStringFile.new_file(
@@ -41,6 +49,15 @@ def carrierwave_file
)
end
end
+
+ def log_params(pipeline_artifact)
+ {
+ project_id: pipeline.project_id,
+ pipeline_id: pipeline.id,
+ pipeline_artifact_id: pipeline_artifact.id,
+ message: "Created code coverage for pipeline."
+ }
+ end
end
end
end
diff --git a/app/workers/ci/pipeline_artifacts/coverage_report_worker.rb b/app/workers/ci/pipeline_artifacts/coverage_report_worker.rb
index 8ee518e3ae6d6dfa0d47ed22a744a16c3bb3befa..5be40f64930f46ea91a0a6e090349f920523bfc1 100644
--- a/app/workers/ci/pipeline_artifacts/coverage_report_worker.rb
+++ b/app/workers/ci/pipeline_artifacts/coverage_report_worker.rb
@@ -15,7 +15,17 @@ class CoverageReportWorker
idempotent!
def perform(pipeline_id)
- Ci::Pipeline.find_by_id(pipeline_id).try do |pipeline|
+ pipeline = Ci::Pipeline.find_by_id(pipeline_id)
+
+ return unless pipeline
+
+ if Feature.enabled?(:ci_child_pipeline_coverage_reports, pipeline.project)
+ pipeline.root_ancestor.try do |root_ancestor_pipeline|
+ next unless root_ancestor_pipeline.self_and_descendants_complete?
+
+ Ci::PipelineArtifacts::CoverageReportService.new(root_ancestor_pipeline).execute
+ end
+ else
Ci::PipelineArtifacts::CoverageReportService.new(pipeline).execute
end
end
diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb
index 801505f0231f4911708ae6c6df5a8bb2c3bd9f9a..b051f646bd4769cd766c97935e9d0302095f0f63 100644
--- a/spec/models/ci/pipeline_artifact_spec.rb
+++ b/spec/models/ci/pipeline_artifact_spec.rb
@@ -196,6 +196,80 @@
end
end
+ describe '.create_or_replace_for_pipeline!' do
+ let_it_be(:pipeline) { create(:ci_empty_pipeline) }
+
+ let(:file_type) { :code_coverage }
+ let(:file) { CarrierWaveStringFile.new_file(file_content: 'content', filename: 'file.json', content_type: 'json') }
+ let(:size) { file['tempfile'].size }
+
+ subject do
+ Ci::PipelineArtifact.create_or_replace_for_pipeline!(
+ pipeline: pipeline,
+ file_type: file_type,
+ file: file,
+ size: size
+ )
+ end
+
+ around do |example|
+ freeze_time { example.run }
+ end
+
+ context 'when there is no existing record' do
+ it 'creates a new pipeline artifact for the given parameters' do
+ expect { subject }.to change { Ci::PipelineArtifact.count }.from(0).to(1)
+
+ expect(subject.code_coverage?).to be(true)
+ expect(subject.pipeline).to eq(pipeline)
+ expect(subject.project_id).to eq(pipeline.project_id)
+ expect(subject.file.filename).to eq(file['filename'])
+ expect(subject.size).to eq(size)
+ expect(subject.file_format).to eq(Ci::PipelineArtifact::REPORT_TYPES[file_type].to_s)
+ expect(subject.expire_at).to eq(Ci::PipelineArtifact::EXPIRATION_DATE.from_now)
+ end
+ end
+
+ context 'when there are existing records with different types' do
+ let!(:existing_artifact) do
+ create(:ci_pipeline_artifact, pipeline: pipeline, file_type: file_type, expire_at: 1.day.from_now)
+ end
+
+ let!(:other_artifact) { create(:ci_pipeline_artifact, pipeline: pipeline, file_type: :code_quality_mr_diff) }
+
+ it 'replaces the existing pipeline artifact record with the given file type' do
+ expect { subject }.not_to change { Ci::PipelineArtifact.count }
+
+ expect(subject.id).not_to eq(existing_artifact.id)
+
+ expect(subject.code_coverage?).to be(true)
+ expect(subject.pipeline).to eq(pipeline)
+ expect(subject.project_id).to eq(pipeline.project_id)
+ expect(subject.file.filename).to eq(file['filename'])
+ expect(subject.size).to eq(size)
+ expect(subject.file_format).to eq(Ci::PipelineArtifact::REPORT_TYPES[file_type].to_s)
+ expect(subject.expire_at).to eq(Ci::PipelineArtifact::EXPIRATION_DATE.from_now)
+ end
+ end
+
+ context 'when ActiveRecordError is raised' do
+ let(:pipeline) { instance_double(Ci::Pipeline, id: 1) }
+ let(:file_type) { :code_coverage }
+ let(:error) { ActiveRecord::ActiveRecordError.new('something went wrong') }
+
+ before do
+ allow(pipeline).to receive(:pipeline_artifacts).and_raise(error)
+ end
+
+ it 'tracks and raise the exception' do
+ expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception)
+ .with(error, { pipeline_id: pipeline.id, file_type: file_type }).and_call_original
+
+ expect { subject }.to raise_error(ActiveRecord::ActiveRecordError, 'something went wrong')
+ end
+ end
+ end
+
describe '#present' do
subject(:presenter) { report.present }
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 31752f300f4bc6ccaad2c80a39831ca2375f50ce..b719adcdace2783aefba72d292792c1867e02481 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -106,6 +106,18 @@
end
end
+ describe 'state machine transitions' do
+ context 'from failed to success' do
+ let_it_be(:pipeline) { create(:ci_empty_pipeline, :failed) }
+
+ it 'schedules CoverageReportWorker' do
+ expect(Ci::PipelineArtifacts::CoverageReportWorker).to receive(:perform_async).with(pipeline.id)
+
+ pipeline.succeed!
+ end
+ end
+ end
+
describe '#set_status' do
let(:pipeline) { build(:ci_empty_pipeline, :created) }
@@ -3760,6 +3772,24 @@ def create_pipeline(status, ref, sha)
end
end
+ describe '#self_and_descendants_complete?' do
+ let_it_be(:pipeline) { create(:ci_pipeline, :success) }
+ let_it_be(:child_pipeline) { create(:ci_pipeline, :success, child_of: pipeline) }
+ let_it_be_with_reload(:grandchild_pipeline) { create(:ci_pipeline, :success, child_of: child_pipeline) }
+
+ context 'when all pipelines in the hierarchy is complete' do
+ it { expect(pipeline.self_and_descendants_complete?).to be(true) }
+ end
+
+ context 'when a pipeline in the hierarchy is not complete' do
+ before do
+ grandchild_pipeline.update!(status: :running)
+ end
+
+ it { expect(pipeline.self_and_descendants_complete?).to be(false) }
+ end
+ end
+
describe '#builds_in_self_and_descendants' do
subject(:builds) { pipeline.builds_in_self_and_descendants }
diff --git a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
index 403afde5da32c83261206d34fde45235dcf0aa97..4b85c52ebce8e316e9669222f712fcedb6394896 100644
--- a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
+++ b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
@@ -2,16 +2,18 @@
require 'spec_helper'
-RSpec.describe ::Ci::PipelineArtifacts::CoverageReportService do
+RSpec.describe Ci::PipelineArtifacts::CoverageReportService do
describe '#execute' do
let_it_be(:project) { create(:project, :repository) }
subject { described_class.new(pipeline).execute }
- shared_examples 'creating a pipeline coverage report' do
+ shared_examples 'creating or updating a pipeline coverage report' do
context 'when pipeline is finished' do
- it 'creates a pipeline artifact' do
- expect { subject }.to change { Ci::PipelineArtifact.count }.from(0).to(1)
+ it 'creates or updates a pipeline artifact' do
+ subject
+
+ expect(pipeline.reload.pipeline_artifacts.count).to eq(1)
end
it 'persists the default file name' do
@@ -22,7 +24,7 @@
expect(file.filename).to eq('code_coverage.json')
end
- it 'sets expire_at to 1 week' do
+ it 'sets expire_at to 1 week from now' do
freeze_time do
subject
@@ -31,13 +33,16 @@
expect(pipeline_artifact.expire_at).to eq(1.week.from_now)
end
end
- end
- context 'when pipeline artifact has already been created' do
- it 'does not raise an error and does not persist the same artifact twice' do
- expect { 2.times { described_class.new(pipeline).execute } }.not_to raise_error
+ it 'logs relevant information' do
+ expect(Gitlab::AppLogger).to receive(:info).with({
+ project_id: project.id,
+ pipeline_id: pipeline.id,
+ pipeline_artifact_id: kind_of(Numeric),
+ message: kind_of(String)
+ })
- expect(Ci::PipelineArtifact.count).to eq(1)
+ subject
end
end
end
@@ -45,21 +50,42 @@
context 'when pipeline has coverage report' do
let!(:pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project) }
- it_behaves_like 'creating a pipeline coverage report'
+ it_behaves_like 'creating or updating a pipeline coverage report'
end
context 'when pipeline has coverage report from child pipeline' do
let!(:pipeline) { create(:ci_pipeline, :success, project: project) }
let!(:child_pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project, child_of: pipeline) }
- it_behaves_like 'creating a pipeline coverage report'
+ it_behaves_like 'creating or updating a pipeline coverage report'
+ end
+
+ context 'when pipeline has existing pipeline artifact for coverage report' do
+ let!(:pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project) }
+ let!(:child_pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project, child_of: pipeline) }
+
+ let!(:pipeline_artifact) do
+ create(:ci_pipeline_artifact, :with_coverage_report, pipeline: pipeline, expire_at: 1.day.from_now)
+ end
+
+ it_behaves_like 'creating or updating a pipeline coverage report'
+
+ context 'when the feature flag is disabled' do
+ before do
+ stub_feature_flags(ci_child_pipeline_coverage_reports: false)
+ end
+
+ it 'does not change existing pipeline artifact' do
+ expect { subject }.not_to change { pipeline_artifact.reload.updated_at }
+ end
+ end
end
context 'when pipeline is running and coverage report does not exist' do
let(:pipeline) { create(:ci_pipeline, :running) }
it 'does not persist data' do
- expect { subject }.not_to change { Ci::PipelineArtifact.count }
+ expect { subject }.not_to change { Ci::PipelineArtifact.count }.from(0)
end
end
end
diff --git a/spec/workers/ci/pipeline_artifacts/coverage_report_worker_spec.rb b/spec/workers/ci/pipeline_artifacts/coverage_report_worker_spec.rb
index 000eda055af2f6e65f443d77c8174e33611543b2..f51e66aa948a820ed3f1dd0dd9d9e91364b985e2 100644
--- a/spec/workers/ci/pipeline_artifacts/coverage_report_worker_spec.rb
+++ b/spec/workers/ci/pipeline_artifacts/coverage_report_worker_spec.rb
@@ -2,28 +2,71 @@
require 'spec_helper'
-RSpec.describe ::Ci::PipelineArtifacts::CoverageReportWorker do
+RSpec.describe Ci::PipelineArtifacts::CoverageReportWorker do
describe '#perform' do
+ let(:pipeline_id) { pipeline.id }
+
subject { described_class.new.perform(pipeline_id) }
context 'when pipeline exists' do
- let(:pipeline) { create(:ci_pipeline) }
- let(:pipeline_id) { pipeline.id }
+ let(:pipeline) { create(:ci_pipeline, :success) }
- it 'calls pipeline report result service' do
- expect_next_instance_of(::Ci::PipelineArtifacts::CoverageReportService) do |create_artifact_service|
- expect(create_artifact_service).to receive(:execute)
+ it 'calls the pipeline coverage report service' do
+ expect_next_instance_of(::Ci::PipelineArtifacts::CoverageReportService, pipeline) do |service|
+ expect(service).to receive(:execute)
end
subject
end
end
+ context 'when the pipeline is part of a hierarchy' do
+ let_it_be(:root_ancestor_pipeline) { create(:ci_pipeline, :success) }
+ let_it_be(:pipeline) { create(:ci_pipeline, :success, child_of: root_ancestor_pipeline) }
+ let_it_be(:another_child_pipeline) { create(:ci_pipeline, :success, child_of: root_ancestor_pipeline) }
+
+ context 'when all pipelines is complete' do
+ it 'calls the pipeline coverage report service on the root ancestor pipeline' do
+ expect_next_instance_of(::Ci::PipelineArtifacts::CoverageReportService, root_ancestor_pipeline) do |service|
+ expect(service).to receive(:execute)
+ end
+
+ subject
+ end
+ end
+
+ context 'when the pipeline hierarchy has incomplete pipeline' do
+ before do
+ another_child_pipeline.update!(status: :running)
+ end
+
+ it 'does not call pipeline coverage report service' do
+ expect(Ci::PipelineArtifacts::CoverageReportService).not_to receive(:new)
+
+ subject
+ end
+ end
+
+ context 'when feature flag is disabled' do
+ before do
+ stub_feature_flags(ci_child_pipeline_coverage_reports: false)
+ end
+
+ it 'calls the pipeline coverage report service on the pipeline' do
+ expect_next_instance_of(::Ci::PipelineArtifacts::CoverageReportService, pipeline) do |service|
+ expect(service).to receive(:execute)
+ end
+
+ subject
+ end
+ end
+ end
+
context 'when pipeline does not exist' do
let(:pipeline_id) { non_existing_record_id }
it 'does not call pipeline create artifact service' do
- expect(Ci::PipelineArtifacts::CoverageReportService).not_to receive(:execute)
+ expect(Ci::PipelineArtifacts::CoverageReportService).not_to receive(:new)
subject
end