Merge branch '344533-new-coverage_report-keyword' into 'master'

Add coverage_report keyword in CI reports

See merge request gitlab-org/gitlab!81378

app/presenters/ci/build_runner_presenter.rb

@@ -64,35 +64,50 @@ module Ci
     def create_archive(artifacts)
       return unless artifacts[:untracked] || artifacts[:paths]
 
-      archive = {
-        artifact_type: :archive,
-        artifact_format: :zip,
-        name: artifacts[:name],
-        untracked: artifacts[:untracked],
-        paths: artifacts[:paths],
-        when: artifacts[:when],
-        expire_in: artifacts[:expire_in]
-      }
-
-      if artifacts.dig(:exclude).present?
-        archive.merge(exclude: artifacts[:exclude])
-      else
-        archive
+      BuildArtifact.for_archive(artifacts).to_h.tap do |artifact|
+        artifact.delete(:exclude) unless artifact[:exclude].present?
       end
     end
 
     def create_reports(reports, expire_in:)
       return unless reports&.any?
 
-      reports.map do |report_type, report_paths|
-        {
-          artifact_type: report_type.to_sym,
-          artifact_format: ::Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(report_type.to_sym),
-          name: ::Ci::JobArtifact::DEFAULT_FILE_NAMES.fetch(report_type.to_sym),
-          paths: report_paths,
+      reports.map { |report| BuildArtifact.for_report(report, expire_in).to_h.compact }
+    end
+
+    BuildArtifact = Struct.new(:name, :untracked, :paths, :exclude, :when, :expire_in, :artifact_type, :artifact_format, keyword_init: true) do
+      def self.for_archive(artifacts)
+        self.new(
+          artifact_type: :archive,
+          artifact_format: :zip,
+          name: artifacts[:name],
+          untracked: artifacts[:untracked],
+          paths: artifacts[:paths],
+          when: artifacts[:when],
+          expire_in: artifacts[:expire_in],
+          exclude: artifacts[:exclude]
+        )
+      end
+
+      def self.for_report(report, expire_in)
+        type, params = report
+
+        if type == :coverage_report
+          artifact_type = params[:coverage_format].to_sym
+          paths = [params[:path]]
+        else
+          artifact_type = type
+          paths = params
+        end
+
+        self.new(
+          artifact_type: artifact_type,
+          artifact_format: ::Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(artifact_type),
+          name: ::Ci::JobArtifact::DEFAULT_FILE_NAMES.fetch(artifact_type),
+          paths: paths,
           when: 'always',
           expire_in: expire_in
-        }
+        )
       end
     end
 

doc/ci/yaml/artifacts_reports.md

@@ -80,9 +80,14 @@ GitLab can display the results of one or more reports in:
 - The [security dashboard](../../user/application_security/security_dashboard/index.md).
 - The [Project Vulnerability report](../../user/application_security/vulnerability_report/index.md).
 
-## `artifacts:reports:cobertura`
+## `artifacts:reports:cobertura` (DEPRECATED)
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/3708) in GitLab 12.9.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/3708) in GitLab 12.9.
+> - [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78132) in GitLab 14.9.
+
+WARNING:
+This feature is in its end-of-life process. It is [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78132) for use in GitLab
+14.8 and replaced with `artifacts:reports:coverage_report`.
 
 The `cobertura` report collects [Cobertura coverage XML files](../../user/project/merge_requests/test_coverage_visualization.md).
 The collected Cobertura coverage reports upload to GitLab as an artifact.
@@ -93,6 +98,28 @@ GitLab can display the results of one or more reports in the merge request
 Cobertura was originally developed for Java, but there are many third-party ports for other languages such as
 JavaScript, Python, and Ruby.
 
+## `artifacts:reports:coverage_report`
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/344533) in GitLab 14.9.
+
+Use `coverage_report` to collect coverage report in Cobertura format, similar to `artifacts:reports:cobertura`.
+
+NOTE:
+`artifacts:reports:coverage_report` cannot be used at the same time with `artifacts:reports:cobertura`.
+
+```yaml
+artifacts:
+  reports:
+    coverage_report:
+      coverage_format: cobertura
+      path: coverage/cobertura-coverage.xml
+```
+
+The collected coverage report is uploaded to GitLab as an artifact.
+
+GitLab can display the results of coverage report in the merge request
+[diff annotations](../../user/project/merge_requests/test_coverage_visualization.md).
+
 ## `artifacts:reports:codequality`
 
 > [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/212499) to GitLab Free in 13.2.

doc/user/project/merge_requests/test_coverage_visualization.md

@@ -28,7 +28,7 @@ between pipeline completion and the visualization loading on the page.
 
 For the coverage analysis to work, you have to provide a properly formatted
 [Cobertura XML](https://cobertura.github.io/cobertura/) report to
-[`artifacts:reports:cobertura`](../../../ci/yaml/artifacts_reports.md#artifactsreportscobertura).
+[`artifacts:reports:cobertura`](../../../ci/yaml/artifacts_reports.md#artifactsreportscobertura-deprecated).
 This format was originally developed for Java, but most coverage analysis frameworks
 for other languages have plugins to add support for it, like:
 

lib/api/entities/ci/job_request/artifacts.rb

@@ -6,7 +6,7 @@ module API
       module JobRequest
         class Artifacts < Grape::Entity
           expose :name
-          expose :untracked
+          expose :untracked, expose_nil: false
           expose :paths
           expose :exclude, expose_nil: false
           expose :when

lib/gitlab/ci/config/entry/reports.rb

@@ -8,6 +8,7 @@ module Gitlab
         # Entry that represents a configuration of job artifacts.
         #
         class Reports < ::Gitlab::Config::Entry::Node
+          include ::Gitlab::Config::Entry::Configurable
           include ::Gitlab::Config::Entry::Validatable
           include ::Gitlab::Config::Entry::Attributable
 
@@ -15,10 +16,13 @@ module Gitlab
             %i[junit codequality sast secret_detection dependency_scanning container_scanning
                dast performance browser_performance load_performance license_scanning metrics lsif
                dotenv cobertura terraform accessibility cluster_applications
-               requirements coverage_fuzzing api_fuzzing cluster_image_scanning].freeze
+               requirements coverage_fuzzing api_fuzzing cluster_image_scanning
+               coverage_report].freeze
 
           attributes ALLOWED_KEYS
 
+          entry :coverage_report, Reports::CoverageReport, description: 'Coverage report configuration.'
+
           validations do
             validates :config, type: Hash
             validates :config, allowed_keys: ALLOWED_KEYS
@@ -47,10 +51,18 @@ module Gitlab
               validates :cluster_applications, array_of_strings_or_string: true # DEPRECATED: https://gitlab.com/gitlab-org/gitlab/-/issues/333441
               validates :requirements, array_of_strings_or_string: true
             end
+
+            validates :config, mutually_exclusive_keys: [:coverage_report, :cobertura]
           end
 
           def value
-            @config.transform_values { |v| Array(v) }
+            @config.transform_values do |value|
+              if value.is_a?(Hash)
+                value
+              else
+                Array(value)
+              end
+            end
           end
         end
       end

lib/gitlab/ci/config/entry/reports/coverage_report.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class Reports
+          class CoverageReport < ::Gitlab::Config::Entry::Node
+            include ::Gitlab::Config::Entry::Validatable
+            include ::Gitlab::Config::Entry::Attributable
+
+            ALLOWED_KEYS = %i[coverage_format path].freeze
+            SUPPORTED_COVERAGE = %w[cobertura].freeze
+
+            attributes ALLOWED_KEYS
+
+            validations do
+              validates :config, type: Hash
+              validates :config, allowed_keys: ALLOWED_KEYS
+
+              with_options(presence: true) do
+                validates :coverage_format, inclusion: { in: SUPPORTED_COVERAGE, message: "must be one of supported formats: #{SUPPORTED_COVERAGE.join(', ')}." }
+                validates :path, type: String
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/config/entry/validators.rb

@@ -39,6 +39,17 @@ module Gitlab
           end
         end
 
+        class MutuallyExclusiveKeysValidator < ActiveModel::EachValidator
+          def validate_each(record, attribute, value)
+            mutually_exclusive_keys = value.try(:keys).to_a & options[:in]
+
+            if mutually_exclusive_keys.length > 1
+              record.errors.add(attribute, "please use only one the following keys: " +
+                mutually_exclusive_keys.join(', '))
+            end
+          end
+        end
+
         class AllowedValuesValidator < ActiveModel::EachValidator
           def validate_each(record, attribute, value)
             unless options[:in].include?(value.to_s)

qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb

@@ -58,6 +58,16 @@ module QA
                     artifacts:
                       paths:
                       - my-artifacts/
+
+                  test-coverage-report:
+                    tags:
+                      - #{executor}
+                    script: mkdir coverage; echo "CONTENTS" > coverage/cobertura.xml
+                    artifacts:
+                      reports:
+                        coverage_report:
+                          coverage_format: cobertura
+                          path: coverage/cobertura.xml
                 YAML
               }
             ]
@@ -71,7 +81,8 @@ module QA
             'test-success': 'passed',
             'test-failure': 'failed',
             'test-tags-mismatch': 'pending',
-            'test-artifacts': 'passed'
+            'test-artifacts': 'passed',
+            'test-coverage-report': 'passed'
           }.each do |job, status|
             Page::Project::Pipeline::Show.perform do |pipeline|
               pipeline.click_job(job)

spec/factories/ci/builds.rb

@@ -497,6 +497,22 @@ FactoryBot.define do
       options { {} }
     end
 
+    trait :coverage_report_cobertura do
+      options do
+        {
+          artifacts: {
+            expire_in: '7d',
+            reports: {
+              coverage_report: {
+                coverage_format: 'cobertura',
+                path: 'cobertura.xml'
+              }
+            }
+          }
+        }
+      end
+    end
+
     # TODO: move Security traits to ee_ci_build
     # https://gitlab.com/gitlab-org/gitlab/-/issues/210486
     trait :dast do

spec/lib/gitlab/ci/config/entry/reports/coverage_report_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Ci::Config::Entry::Reports::CoverageReport do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when it is valid' do
+      let(:config) { { coverage_format: 'cobertura', path: 'cobertura-coverage.xml' } }
+
+      it { expect(entry).to be_valid }
+
+      it { expect(entry.value).to eq(config) }
+    end
+
+    context 'with unsupported coverage format' do
+      let(:config) { { coverage_format: 'jacoco', path: 'jacoco.xml' } }
+
+      it { expect(entry).not_to be_valid }
+
+      it { expect(entry.errors).to include /format must be one of supported formats/ }
+    end
+
+    context 'without coverage format' do
+      let(:config) { { path: 'cobertura-coverage.xml' } }
+
+      it { expect(entry).not_to be_valid }
+
+      it { expect(entry.errors).to include /format can't be blank/ }
+    end
+
+    context 'without path' do
+      let(:config) { { coverage_format: 'cobertura' } }
+
+      it { expect(entry).not_to be_valid }
+
+      it { expect(entry.errors).to include /path can't be blank/ }
+    end
+
+    context 'with invalid path' do
+      let(:config) { { coverage_format: 'cobertura', path: 123 } }
+
+      it { expect(entry).not_to be_valid }
+
+      it { expect(entry.errors).to include /path should be a string/ }
+    end
+
+    context 'with unknown keys' do
+      let(:config) { { coverage_format: 'cobertura', path: 'cobertura-coverage.xml', foo: :bar } }
+
+      it { expect(entry).not_to be_valid }
+
+      it { expect(entry.errors).to include /contains unknown keys/ }
+    end
+  end
+end

spec/lib/gitlab/ci/config/entry/reports_spec.rb

@@ -6,12 +6,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Reports do
   let(:entry) { described_class.new(config) }
 
   describe 'validates ALLOWED_KEYS' do
-    let(:artifact_file_types) { Ci::JobArtifact.file_types }
-
-    described_class::ALLOWED_KEYS.each do |keyword, _|
-      it "expects #{keyword} to be an artifact file_type" do
-        expect(artifact_file_types).to include(keyword)
-      end
+    it "expects ALLOWED_KEYS to be an artifact file_type or coverage_report" do
+      expect(Ci::JobArtifact.file_types.keys.map(&:to_sym) + [:coverage_report]).to include(*described_class::ALLOWED_KEYS)
     end
   end
 
@@ -68,6 +64,45 @@ RSpec.describe Gitlab::Ci::Config::Entry::Reports do
           it_behaves_like 'a valid entry', params[:keyword], params[:file]
         end
       end
+
+      context 'when coverage_report is specified' do
+        let(:coverage_format) { :cobertura }
+        let(:filename) { 'cobertura-coverage.xml' }
+        let(:coverage_report) { { path: filename, coverage_format: coverage_format } }
+        let(:config) { { coverage_report: coverage_report } }
+
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+
+        it 'returns artifacts configuration' do
+          expect(entry.value).to eq(config)
+        end
+
+        context 'and another report is specified' do
+          let(:config) { { coverage_report: coverage_report, dast: 'gl-dast-report.json' } }
+
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
+
+          it 'returns artifacts configuration' do
+            expect(entry.value).to eq({ coverage_report: coverage_report, dast: ['gl-dast-report.json'] })
+          end
+        end
+
+        context 'and a direct coverage report format is specified' do
+          let(:config) { { coverage_report: coverage_report, cobertura: 'cobertura-coverage.xml' } }
+
+          it 'is not valid' do
+            expect(entry).not_to be_valid
+          end
+
+          it 'reports error' do
+            expect(entry.errors).to include /please use only one the following keys: coverage_report, cobertura/
+          end
+        end
+      end
     end
 
     context 'when entry value is not correct' do

spec/lib/gitlab/config/entry/validators_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Config::Entry::Validators do
+  let(:klass) do
+    Class.new do
+      include ActiveModel::Validations
+      include Gitlab::Config::Entry::Validators
+    end
+  end
+
+  let(:instance) { klass.new }
+
+  describe described_class::MutuallyExclusiveKeysValidator do
+    using RSpec::Parameterized::TableSyntax
+
+    before do
+      klass.instance_eval do
+        validates :config, mutually_exclusive_keys: [:foo, :bar]
+      end
+
+      allow(instance).to receive(:config).and_return(config)
+    end
+
+    where(:context, :config, :valid_result) do
+      'with mutually exclusive keys' | { foo: 1, bar: 2 } | false
+      'without mutually exclusive keys' | { foo: 1 } | true
+      'without mutually exclusive keys' | { bar: 1 } | true
+      'with other keys' | { foo: 1, baz: 2 } | true
+    end
+
+    with_them do
+      it 'validates the instance' do
+        expect(instance.valid?).to be(valid_result)
+
+        unless valid_result
+          expect(instance.errors.messages_for(:config)).to include /please use only one the following keys: foo, bar/
+        end
+      end
+    end
+  end
+end

spec/presenters/ci/build_runner_presenter_spec.rb

@@ -78,16 +78,72 @@ RSpec.describe Ci::BuildRunnerPresenter do
               artifact_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(file_type),
               paths: [filename],
               when: 'always'
-            }
+            }.compact
           end
 
           it 'presents correct hash' do
-            expect(presenter.artifacts.first).to include(report_expectation)
+            expect(presenter.artifacts).to contain_exactly(report_expectation)
           end
         end
       end
     end
 
+    context 'when a specific coverage_report type is given' do
+      let(:coverage_format) { :cobertura }
+      let(:filename) { 'cobertura-coverage.xml' }
+      let(:coverage_report) { { path: filename, coverage_format: coverage_format } }
+      let(:report) { { coverage_report: coverage_report } }
+      let(:build) { create(:ci_build, options: { artifacts: { reports: report } }) }
+
+      let(:expected_coverage_report) do
+        {
+          name: filename,
+          artifact_type: coverage_format,
+          artifact_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(coverage_format),
+          paths: [filename],
+          when: 'always'
+        }
+      end
+
+      it 'presents the coverage report hash with the coverage format' do
+        expect(presenter.artifacts).to contain_exactly(expected_coverage_report)
+      end
+    end
+
+    context 'when a specific coverage_report type is given with another report type' do
+      let(:coverage_format) { :cobertura }
+      let(:coverage_filename) { 'cobertura-coverage.xml' }
+      let(:coverage_report) { { path: coverage_filename, coverage_format: coverage_format } }
+      let(:ds_filename) { 'gl-dependency-scanning-report.json' }
+
+      let(:report) { { coverage_report: coverage_report, dependency_scanning: [ds_filename] } }
+      let(:build) { create(:ci_build, options: { artifacts: { reports: report } }) }
+
+      let(:expected_coverage_report) do
+        {
+          name: coverage_filename,
+          artifact_type: coverage_format,
+          artifact_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(coverage_format),
+          paths: [coverage_filename],
+          when: 'always'
+        }
+      end
+
+      let(:expected_ds_report) do
+        {
+          name: ds_filename,
+          artifact_type: :dependency_scanning,
+          artifact_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(:dependency_scanning),
+          paths: [ds_filename],
+          when: 'always'
+        }
+      end
+
+      it 'presents both reports' do
+        expect(presenter.artifacts).to contain_exactly(expected_coverage_report, expected_ds_report)
+      end
+    end
+
     context "when option has both archive and reports specification" do
       let(:report) { { junit: ['junit.xml'] } }
       let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: report } }) }

spec/requests/api/ci/runner/jobs_request_post_spec.rb

@@ -611,6 +611,40 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
             end
           end
 
+          context 'when job has code coverage report' do
+            let(:job) do
+              create(:ci_build, :pending, :queued, :coverage_report_cobertura,
+                     pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0)
+            end
+
+            let(:expected_artifacts) do
+              [
+                {
+                  'name' => 'cobertura-coverage.xml',
+                  'paths' => ['cobertura.xml'],
+                  'when' => 'always',
+                  'expire_in' => '7d',
+                  "artifact_type" => "cobertura",
+                  "artifact_format" => "gzip"
+                }
+              ]
+            end
+
+            it 'returns job with the correct artifact specification' do
+              request_job info: { platform: :darwin, features: { upload_multiple_artifacts: true } }
+
+              expect(response).to have_gitlab_http_status(:created)
+              expect(response.headers['Content-Type']).to eq('application/json')
+              expect(response.headers).not_to have_key('X-GitLab-Last-Update')
+              expect(runner.reload.platform).to eq('darwin')
+              expect(json_response['id']).to eq(job.id)
+              expect(json_response['token']).to eq(job.token)
+              expect(json_response['job_info']).to eq(expected_job_info)
+              expect(json_response['git_info']).to eq(expected_git_info)
+              expect(json_response['artifacts']).to eq(expected_artifacts)
+            end
+          end
+
           context 'when triggered job is available' do
             let(:expected_variables) do
               [{ 'key' => 'CI_JOB_NAME', 'value' => 'spinach', 'public' => true, 'masked' => false },

spec/services/ci/create_pipeline_service/artifacts_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Ci::CreatePipelineService do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { project.first_owner }
+
+  let(:ref) { 'refs/heads/master' }
+  let(:source) { :push }
+
+  let(:service) { described_class.new(project, user, { ref: ref }) }
+  let(:pipeline) { service.execute(source).payload }
+
+  describe 'artifacts:' do
+    before do
+      stub_ci_pipeline_yaml_file(config)
+      allow_next_instance_of(Ci::BuildScheduleWorker) do |instance|
+        allow(instance).to receive(:perform).and_return(true)
+      end
+    end
+
+    describe 'reports:' do
+      context 'with valid config' do
+        let(:config) do
+          <<~YAML
+          test-job:
+            script: "echo 'hello world' > cobertura.xml"
+            artifacts:
+              reports:
+                coverage_report:
+                  coverage_format: 'cobertura'
+                  path: 'cobertura.xml'
+
+          dependency-scanning-job:
+            script: "echo 'hello world' > gl-dependency-scanning-report.json"
+            artifacts:
+              reports:
+                dependency_scanning: 'gl-dependency-scanning-report.json'
+          YAML
+        end
+
+        it 'creates pipeline with builds' do
+          expect(pipeline).to be_persisted
+          expect(pipeline).not_to have_yaml_errors
+          expect(pipeline.builds.pluck(:name)).to contain_exactly('test-job', 'dependency-scanning-job')
+        end
+      end
+
+      context 'with invalid config' do
+        let(:config) do
+          <<~YAML
+          test-job:
+            script: "echo 'hello world' > cobertura.xml"
+            artifacts:
+              reports:
+                foo: 'bar'
+          YAML
+        end
+
+        it 'creates pipeline with yaml errors' do
+          expect(pipeline).to be_persisted
+          expect(pipeline).to have_yaml_errors
+        end
+      end
+    end
+  end
+end