Merge branch 'revert-7204aec7' into 'master'

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

See merge request gitlab-org/gitlab!82627

app/presenters/ci/build_runner_presenter.rb

@@ -64,50 +64,35 @@ module Ci
     def create_archive(artifacts)
       return unless artifacts[:untracked] || artifacts[:paths]
 
-      BuildArtifact.for_archive(artifacts).to_h.tap do |artifact|
-        artifact.delete(:exclude) unless artifact[:exclude].present?
+      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
       end
     end
 
     def create_reports(reports, expire_in:)
       return unless reports&.any?
 
-      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,
+      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,
           when: 'always',
           expire_in: expire_in
-        )
+        }
       end
     end
 

doc/ci/yaml/artifacts_reports.md

@@ -80,14 +80,9 @@ 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` (DEPRECATED)
+## `artifacts:reports:cobertura`
 
-> - [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`.
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/3708) in GitLab 12.9.
 
 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.
@@ -98,28 +93,6 @@ 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-deprecated).
+[`artifacts:reports:cobertura`](../../../ci/yaml/artifacts_reports.md#artifactsreportscobertura).
 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_nil: false
+          expose :untracked
           expose :paths
           expose :exclude, expose_nil: false
           expose :when

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

@@ -8,7 +8,6 @@ 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
 
@@ -16,13 +15,10 @@ 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
-               coverage_report].freeze
+               requirements coverage_fuzzing api_fuzzing cluster_image_scanning].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
@@ -51,18 +47,10 @@ 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 do |value|
-              if value.is_a?(Hash)
-                value
-              else
-                Array(value)
-              end
-            end
+            @config.transform_values { |v| Array(v) }
           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,17 +39,6 @@ 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,16 +58,6 @@ 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
               }
             ]
@@ -81,8 +71,7 @@ module QA
             'test-success': 'passed',
             'test-failure': 'failed',
             'test-tags-mismatch': 'pending',
-            'test-artifacts': 'passed',
-            'test-coverage-report': 'passed'
+            'test-artifacts': 'passed'
           }.each do |job, status|
             Page::Project::Pipeline::Show.perform do |pipeline|
               pipeline.click_job(job)

spec/factories/ci/builds.rb

@@ -497,22 +497,6 @@ 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,8 +6,12 @@ RSpec.describe Gitlab::Ci::Config::Entry::Reports do
   let(:entry) { described_class.new(config) }
 
   describe 'validates ALLOWED_KEYS' do
-    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)
+    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
     end
   end
 
@@ -64,45 +68,6 @@ 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,72 +78,16 @@ 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).to contain_exactly(report_expectation)
+            expect(presenter.artifacts.first).to include(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,40 +611,6 @@ 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