Merge branch 'mo-swith-coverage-repor-with-object-storage' into 'master'

Swith coverage report with ObjectStorage

See merge request gitlab-org/gitlab!39721

app/models/ci/pipeline_artifact.rb

@@ -40,5 +40,9 @@ module Ci
     def self.has_code_coverage?
       where(file_type: :code_coverage).exists?
     end
+
+    def self.find_with_code_coverage
+      find_by(file_type: :code_coverage)
+    end
   end
 end

app/models/merge_request.rb

@@ -1344,7 +1344,7 @@ class MergeRequest < ApplicationRecord
   def has_coverage_reports?
     return false unless Feature.enabled?(:coverage_report_view, project)
 
-    actual_head_pipeline&.has_reports?(Ci::JobArtifact.coverage_reports)
+    actual_head_pipeline&.pipeline_artifacts&.has_code_coverage?
   end
 
   def has_terraform_reports?

app/services/ci/generate_coverage_reports_service.rb

@@ -12,7 +12,7 @@ module Ci
       {
         status: :parsed,
         key: key(base_pipeline, head_pipeline),
-        data: head_pipeline.coverage_reports.pick(merge_request.new_paths)
+        data: Gitlab::Ci::Pipeline::Artifact::CodeCoverage.new(head_pipeline.pipeline_artifacts.find_with_code_coverage).for_files(merge_request.new_paths)
       }
     rescue => e
       Gitlab::ErrorTracking.track_exception(e, project_id: project.id)

lib/gitlab/ci/pipeline/artifact/code_coverage.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Pipeline
+      module Artifact
+        class CodeCoverage
+          def initialize(pipeline_artifact)
+            @pipeline_artifact = pipeline_artifact
+          end
+
+          def for_files(filenames)
+            coverage_files = raw_report["files"].select { |key| filenames.include?(key) }
+
+            { files: coverage_files }
+          end
+
+          private
+
+          def raw_report
+            @raw_report ||= Gitlab::Json.parse(@pipeline_artifact.file.read)
+          end
+        end
+      end
+    end
+  end
+end

spec/factories/ci/pipeline_artifacts.rb

@@ -24,5 +24,15 @@ FactoryBot.define do
         )
       end
     end
+
+    trait :with_code_coverage_with_multiple_files do
+      after(:build) do |artifact, _evaluator|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/pipeline_artifacts/code_coverage_with_multiple_files.json'), 'application/json'
+        )
+      end
+
+      size { file.size }
+    end
   end
 end

spec/factories/ci/pipelines.rb

@@ -121,6 +121,12 @@ FactoryBot.define do
         end
       end
 
+      trait :with_coverage_report_artifact do
+        after(:build) do |pipeline, evaluator|
+          pipeline.pipeline_artifacts << build(:ci_pipeline_artifact, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
       trait :with_terraform_reports do
         status { :success }
 

spec/factories/merge_requests.rb

@@ -169,7 +169,7 @@ FactoryBot.define do
         merge_request.head_pipeline = build(
           :ci_pipeline,
           :success,
-          :with_coverage_reports,
+          :with_coverage_report_artifact,
           project: merge_request.source_project,
           ref: merge_request.source_branch,
           sha: merge_request.diff_head_sha)

spec/fixtures/pipeline_artifacts/code_coverage_with_multiple_files.json

+{
+  "files": {
+    "file_a.rb": {
+      "1": 1,
+      "2": 1,
+      "3": 1
+    },
+    "file_b.rb": {
+      "1": 0,
+      "2": 0,
+      "3": 0
+    }
+  }
+}

spec/lib/gitlab/ci/pipeline/artifact/code_coverage_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Pipeline::Artifact::CodeCoverage do
+  let(:pipeline_artifact) { create(:ci_pipeline_artifact, :with_code_coverage_with_multiple_files) }
+  let(:code_coverage) { described_class.new(pipeline_artifact) }
+
+  describe '#for_files' do
+    subject { code_coverage.for_files(filenames) }
+
+    context 'when code coverage has data' do
+      context 'when filenames is empty' do
+        let(:filenames) { %w() }
+
+        it 'returns hash without coverage' do
+          expect(subject).to match(files: {})
+        end
+      end
+
+      context 'when filenames do not match code coverage data' do
+        let(:filenames) { %w(demo.rb) }
+
+        it 'returns hash without coverage' do
+          expect(subject).to match(files: {})
+        end
+      end
+
+      context 'when filenames matches code coverage data' do
+        context 'when asking for one filename' do
+          let(:filenames) { %w(file_a.rb) }
+
+          it 'returns coverage for the given filename' do
+            expect(subject).to match(files: { "file_a.rb" => { "1" => 1, "2" => 1, "3" => 1 } })
+          end
+        end
+
+        context 'when asking for multiple filenames' do
+          let(:filenames) { %w(file_a.rb file_b.rb) }
+
+          it 'returns coverage for a the given filenames' do
+            expect(subject).to match(
+              files: {
+                "file_a.rb" => {
+                  "1" => 1,
+                  "2" => 1,
+                  "3" => 1
+                },
+                "file_b.rb" => {
+                  "1" => 0,
+                  "2" => 0,
+                  "3" => 0
+                }
+              }
+            )
+          end
+        end
+      end
+    end
+  end
+end

spec/models/ci/pipeline_artifact_spec.rb

@@ -91,4 +91,22 @@ RSpec.describe Ci::PipelineArtifact, type: :model do
       end
     end
   end
+
+  describe '.find_with_code_coverage' do
+    subject { Ci::PipelineArtifact.find_with_code_coverage }
+
+    context 'when pipeline artifact has a coverage report' do
+      let!(:coverage_report) { create(:ci_pipeline_artifact) }
+
+      it 'returns a pipeline artifact with a code coverage' do
+        expect(subject.file_type).to eq('code_coverage')
+      end
+    end
+
+    context 'when pipeline artifact does not have a coverage report' do
+      it 'returns nil' do
+        expect(subject).to be_nil
+      end
+    end
+  end
 end

spec/models/merge_request_spec.rb

@@ -1890,12 +1890,6 @@ RSpec.describe MergeRequest do
     subject { merge_request.find_coverage_reports }
 
     context 'when head pipeline has coverage reports' do
-      let!(:job) do
-        create(:ci_build, options: { artifacts: { reports: { cobertura: ['cobertura-coverage.xml'] } } }, pipeline: pipeline)
-      end
-
-      let!(:artifacts_metadata) { create(:ci_job_artifact, :metadata, job: job) }
-
       context 'when reactive cache worker is parsing results asynchronously' do
         it 'returns status' do
           expect(subject[:status]).to eq(:parsing)

spec/services/ci/generate_coverage_reports_service_spec.rb

@@ -15,7 +15,11 @@ RSpec.describe Ci::GenerateCoverageReportsService do
       let!(:head_pipeline) { merge_request.head_pipeline }
       let!(:base_pipeline) { nil }
 
-      it 'returns status and data' do
+      it 'returns status and data', :aggregate_failures do
+        expect_next_instance_of(Gitlab::Ci::Pipeline::Artifact::CodeCoverage) do |instance|
+          expect(instance).to receive(:for_files).with(merge_request.new_paths).and_call_original
+        end
+
         expect(subject[:status]).to eq(:parsed)
         expect(subject[:data]).to eq(files: {})
       end
@@ -28,8 +32,7 @@ RSpec.describe Ci::GenerateCoverageReportsService do
       let!(:base_pipeline) { nil }
 
       before do
-        build = create(:ci_build, pipeline: head_pipeline, project: head_pipeline.project)
-        create(:ci_job_artifact, :coverage_with_corrupted_data, job: build, project: project)
+        head_pipeline.pipeline_artifacts.destroy_all # rubocop: disable Cop/DestroyAll
       end
 
       it 'returns status and error message' do