9 files changed, 201 insertions, 99 deletions

diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb
index f028803ca74..ee79f0ae5fd 100644
--- a/spec/factories/ci/job_artifacts.rb
+++ b/spec/factories/ci/job_artifacts.rb
@@ -14,6 +14,33 @@ FactoryBot.define do
       artifact.project ||= artifact.job.project
     end
 
+    trait :raw do
+      file_format :raw
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/trace/sample_trace'), 'text/plain')
+      end
+    end
+
+    trait :zip do
+      file_format :zip
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip')
+      end
+    end
+
+    trait :gzip do
+      file_format :gzip
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), 'application/x-gzip')
+      end
+    end
+
     trait :archive do
       file_type :archive
       file_format :zip
diff --git a/spec/lib/gitlab/ci/config/entry/reports_spec.rb b/spec/lib/gitlab/ci/config/entry/reports_spec.rb
index b3a3a6bee1d..e5fc36b5d7a 100644
--- a/spec/lib/gitlab/ci/config/entry/reports_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/reports_spec.rb
@@ -3,27 +3,53 @@ require 'spec_helper'
 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
+    end
+  end
+
   describe 'validation' do
     context 'when entry config value is correct' do
-      let(:config) { { junit: %w[junit.xml] } }
+      using RSpec::Parameterized::TableSyntax
 
-      describe '#value' do
-        it 'returns artifacs configuration' do
-          expect(entry.value).to eq config
+      shared_examples 'a valid entry' do |keyword, file|
+        describe '#value' do
+          it 'returns artifacs configuration' do
+            expect(entry.value).to eq({ "#{keyword}": [file] } )
+          end
         end
-      end
 
-      describe '#valid?' do
-        it 'is valid' do
-          expect(entry).to be_valid
+        describe '#valid?' do
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
         end
       end
 
-      context 'when value is not array' do
-        let(:config) { { junit: 'junit.xml' } }
+      where(:keyword, :file) do
+        :junit | 'junit.xml'
+        :sast | 'gl-sast-report.json'
+        :dependency_scanning | 'gl-dependency-scanning-report.json'
+        :container_scanning | 'gl-container-scanning-report.json'
+        :dast | 'gl-dast-report.json'
+      end
+
+      with_them do
+        context 'when value is an array' do
+          let(:config) { { "#{keyword}": [file] } }
 
-        it 'converts to array' do
-          expect(entry.value).to eq({ junit: ['junit.xml'] } )
+          it_behaves_like 'a valid entry', params[:keyword], params[:file]
+        end
+
+        context 'when value is not array' do
+          let(:config) { { "#{keyword}": file } }
+
+          it_behaves_like 'a valid entry', params[:keyword], params[:file]
         end
       end
     end
@@ -31,11 +57,13 @@ describe Gitlab::Ci::Config::Entry::Reports do
     context 'when entry value is not correct' do
       describe '#errors' do
         context 'when value of attribute is invalid' do
-          let(:config) { { junit: 10 } }
+          where(key: described_class::ALLOWED_KEYS) do
+            let(:config) { { "#{key}": 10 } }
 
-          it 'reports error' do
-            expect(entry.errors)
-              .to include 'reports junit should be an array of strings or a string'
+            it 'reports error' do
+              expect(entry.errors)
+                .to include "reports #{key} should be an array of strings or a string"
+            end
           end
         end
 
diff --git a/spec/lib/gitlab/ci/parsers/junit_spec.rb b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
index 47497f06229..a49402c7398 100644
--- a/spec/lib/gitlab/ci/parsers/junit_spec.rb
+++ b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
@@ -1,6 +1,6 @@
 require 'fast_spec_helper'
 
-describe Gitlab::Ci::Parsers::Junit do
+describe Gitlab::Ci::Parsers::Test::Junit do
   describe '#parse!' do
     subject { described_class.new.parse!(junit, test_suite) }
 
diff --git a/spec/lib/gitlab/ci/parsers_spec.rb b/spec/lib/gitlab/ci/parsers/test_spec.rb
index 2fa83c4abae..0b85b432677 100644
--- a/spec/lib/gitlab/ci/parsers_spec.rb
+++ b/spec/lib/gitlab/ci/parsers/test_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Ci::Parsers do
+describe Gitlab::Ci::Parsers::Test do
   describe '.fabricate!' do
     subject { described_class.fabricate!(file_type) }
 
@@ -16,7 +16,7 @@ describe Gitlab::Ci::Parsers do
       let(:file_type) { 'undefined' }
 
       it 'raises an error' do
-        expect { subject }.to raise_error(NameError)
+        expect { subject }.to raise_error(Gitlab::Ci::Parsers::Test::ParserNotFoundError)
       end
     end
   end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index dbebda20ce0..e82d93d5935 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -176,9 +176,7 @@ describe Ci::Build do
       it 'does not execute a query for selecting job artifact one by one' do
         recorded = ActiveRecord::QueryRecorder.new do
           subject.each do |build|
-            Ci::JobArtifact::TEST_REPORT_FILE_TYPES.each do |file_type|
-              build.public_send("job_artifacts_#{file_type}").file.exists?
-            end
+            build.job_artifacts.map { |a| a.file.exists? }
           end
         end
 
@@ -550,44 +548,22 @@ describe Ci::Build do
     end
   end
 
-  describe '#has_test_reports?' do
-    subject { build.has_test_reports? }
+  describe '#has_job_artifacts?' do
+    subject { build.has_job_artifacts? }
 
-    context 'when build has a test report' do
-      let(:build) { create(:ci_build, :test_reports) }
+    context 'when build has a job artifact' do
+      let(:build) { create(:ci_build, :artifacts) }
 
       it { is_expected.to be_truthy }
     end
 
-    context 'when build does not have test reports' do
-      let(:build) { create(:ci_build, :artifacts) }
+    context 'when build does not have job artifacts' do
+      let(:build) { create(:ci_build, :legacy_artifacts) }
 
       it { is_expected.to be_falsy }
     end
   end
 
-  describe '#erase_test_reports!' do
-    subject { build.erase_test_reports! }
-
-    context 'when build has a test report' do
-      let!(:build) { create(:ci_build, :test_reports) }
-
-      it 'removes a test report' do
-        subject
-
-        expect(build.has_test_reports?).to be_falsy
-      end
-    end
-
-    context 'when build does not have test reports' do
-      let!(:build) { create(:ci_build, :artifacts) }
-
-      it 'does not erase anything' do
-        expect { subject }.not_to change { Ci::JobArtifact.count }
-      end
-    end
-  end
-
   describe '#has_old_trace?' do
     subject { build.has_old_trace? }
 
@@ -850,8 +826,8 @@ describe Ci::Build do
         expect(build.artifacts_metadata.exists?).to be_falsy
       end
 
-      it 'removes test reports' do
-        expect(build.job_artifacts.test_reports.count).to eq(0)
+      it 'removes all job_artifacts' do
+        expect(build.job_artifacts.count).to eq(0)
       end
 
       it 'erases build trace in trace file' do
@@ -1022,6 +998,32 @@ describe Ci::Build do
     end
   end
 
+  describe '#erase_erasable_artifacts!' do
+    let!(:build) { create(:ci_build, :success) }
+
+    subject { build.erase_erasable_artifacts! }
+
+    before do
+      Ci::JobArtifact.file_types.keys.each do |file_type|
+        create(:ci_job_artifact, job: build, file_type: file_type, file_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS[file_type.to_sym])
+      end
+    end
+
+    it "erases erasable artifacts" do
+      subject
+
+      expect(build.job_artifacts.erasable).to be_empty
+    end
+
+    it "keeps non erasable artifacts" do
+      subject
+
+      Ci::JobArtifact::NON_ERASABLE_FILE_TYPES.each do |file_type|
+        expect(build.send("job_artifacts_#{file_type}")).not_to be_nil
+      end
+    end
+  end
+
   describe '#first_pending' do
     let!(:first) { create(:ci_build, pipeline: pipeline, status: 'pending', created_at: Date.yesterday) }
     let!(:second) { create(:ci_build, pipeline: pipeline, status: 'pending') }
@@ -2844,16 +2846,10 @@ describe Ci::Build do
         end
 
         it 'raises an error' do
-          expect { subject }.to raise_error(Gitlab::Ci::Parsers::Junit::JunitParserError)
+          expect { subject }.to raise_error(Gitlab::Ci::Parsers::Test::Junit::JunitParserError)
         end
       end
     end
-
-    context 'when build does not have test reports' do
-      it 'raises an error' do
-        expect { subject }.to raise_error(NoMethodError)
-      end
-    end
   end
 
   describe '#artifacts_metadata_entry' do
diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb
index 1bf338f4c70..2f449d617be 100644
--- a/spec/models/ci/job_artifact_spec.rb
+++ b/spec/models/ci/job_artifact_spec.rb
@@ -31,6 +31,22 @@ describe Ci::JobArtifact do
     end
   end
 
+  describe '.erasable' do
+    subject { described_class.erasable }
+
+    context 'when there is am erasable artifact' do
+      let!(:artifact) { create(:ci_job_artifact, :junit) }
+
+      it { is_expected.to eq([artifact]) }
+    end
+
+    context 'when there are no erasable artifacts' do
+      let!(:artifact) { create(:ci_job_artifact, :trace) }
+
+      it { is_expected.to be_empty }
+    end
+  end
+
   describe 'callbacks' do
     subject { create(:ci_job_artifact, :archive) }
 
@@ -106,34 +122,46 @@ describe Ci::JobArtifact do
   describe 'validates file format' do
     subject { artifact }
 
-    context 'when archive type with zip format' do
-      let(:artifact) { build(:ci_job_artifact, :archive, file_format: :zip) }
+    described_class::TYPE_AND_FORMAT_PAIRS.except(:trace).each do |file_type, file_format|
+      context "when #{file_type} type with #{file_format} format" do
+        let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: file_format) }
 
-      it { is_expected.to be_valid }
-    end
+        it { is_expected.to be_valid }
+      end
 
-    context 'when archive type with gzip format' do
-      let(:artifact) { build(:ci_job_artifact, :archive, file_format: :gzip) }
+      context "when #{file_type} type without format specification" do
+        let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: nil) }
 
-      it { is_expected.not_to be_valid }
-    end
+        it { is_expected.not_to be_valid }
+      end
 
-    context 'when archive type without format specification' do
-      let(:artifact) { build(:ci_job_artifact, :archive, file_format: nil) }
+      context "when #{file_type} type with other formats" do
+        described_class.file_formats.except(file_format).values.each do |other_format|
+          let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: other_format) }
 
-      it { is_expected.not_to be_valid }
+          it { is_expected.not_to be_valid }
+        end
+      end
     end
+  end
 
-    context 'when junit type with zip format' do
-      let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) }
+  describe 'validates DEFAULT_FILE_NAMES' do
+    subject { described_class::DEFAULT_FILE_NAMES }
 
-      it { is_expected.not_to be_valid }
+    described_class.file_types.each do |file_type, _|
+      it "expects #{file_type} to be included" do
+        is_expected.to include(file_type.to_sym)
+      end
     end
+  end
 
-    context 'when junit type with gzip format' do
-      let(:artifact) { build(:ci_job_artifact, :junit, file_format: :gzip) }
+  describe 'validates TYPE_AND_FORMAT_PAIRS' do
+    subject { described_class::TYPE_AND_FORMAT_PAIRS }
 
-      it { is_expected.to be_valid }
+    described_class.file_types.each do |file_type, _|
+      it "expects #{file_type} to be included" do
+        expect(described_class.file_formats).to include(subject[file_type.to_sym])
+      end
     end
   end
 
diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb
index e7019b990dd..a42d1f3d399 100644
--- a/spec/presenters/ci/build_runner_presenter_spec.rb
+++ b/spec/presenters/ci/build_runner_presenter_spec.rb
@@ -3,7 +3,6 @@ require 'spec_helper'
 describe Ci::BuildRunnerPresenter do
   let(:presenter) { described_class.new(build) }
   let(:archive) { { paths: ['sample.txt'] } }
-  let(:junit) { { junit: ['junit.xml'] } }
 
   let(:archive_expectation) do
     {
@@ -14,16 +13,6 @@ describe Ci::BuildRunnerPresenter do
     }
   end
 
-  let(:junit_expectation) do
-    {
-      name: 'junit.xml',
-      artifact_type: :junit,
-      artifact_format: :gzip,
-      paths: ['junit.xml'],
-      when: 'always'
-    }
-  end
-
   describe '#artifacts' do
     context "when option contains archive-type artifacts" do
       let(:build) { create(:ci_build, options: { artifacts: archive } ) }
@@ -49,20 +38,44 @@ describe Ci::BuildRunnerPresenter do
       end
     end
 
-    context "when option has 'junit' keyword" do
-      let(:build) { create(:ci_build, options: { artifacts: { reports: junit } } ) }
+    context "with reports" do
+      Ci::JobArtifact::DEFAULT_FILE_NAMES.each do |file_type, filename|
+        let(:report) { { "#{file_type}": [filename] } }
+        let(:build) { create(:ci_build, options: { artifacts: { reports: report } } ) }
+
+        let(:report_expectation) do
+          {
+             name: filename,
+             artifact_type: :"#{file_type}",
+             artifact_format: :gzip,
+             paths: [filename],
+             when: 'always'
+          }
+        end
 
-      it 'presents correct hash' do
-        expect(presenter.artifacts.first).to include(junit_expectation)
+        it 'presents correct hash' do
+          expect(presenter.artifacts.first).to include(report_expectation)
+        end
       end
     end
 
     context "when option has both archive and reports specification" do
-      let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: junit } } ) }
+      let(:report) { { junit: ['junit.xml'] } }
+      let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: report } } ) }
+
+      let(:report_expectation) do
+        {
+          name: 'junit.xml',
+          artifact_type: :junit,
+          artifact_format: :gzip,
+          paths: ['junit.xml'],
+          when: 'always'
+        }
+      end
 
       it 'presents correct hash' do
         expect(presenter.artifacts.first).to include(archive_expectation)
-        expect(presenter.artifacts.second).to include(junit_expectation)
+        expect(presenter.artifacts.second).to include(report_expectation)
       end
 
       context "when archive specifies 'expire_in' keyword" do
@@ -70,7 +83,7 @@ describe Ci::BuildRunnerPresenter do
 
         it 'inherits expire_in from archive' do
           expect(presenter.artifacts.first).to include({ **archive_expectation, expire_in: '3 mins 4 sec' })
-          expect(presenter.artifacts.second).to include({ **junit_expectation, expire_in: '3 mins 4 sec' })
+          expect(presenter.artifacts.second).to include({ **report_expectation, expire_in: '3 mins 4 sec' })
         end
       end
     end
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index 6adbbb40489..8770365c893 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -721,7 +721,7 @@ describe API::Jobs do
         expect(job.trace.exist?).to be_falsy
         expect(job.artifacts_file.exists?).to be_falsy
         expect(job.artifacts_metadata.exists?).to be_falsy
-        expect(job.has_test_reports?).to be_falsy
+        expect(job.has_job_artifacts?).to be_falsy
       end
 
       it 'updates job' do
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index 18d52082399..951c0b16a68 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -24,7 +24,9 @@ describe Ci::RetryBuildService do
        artifacts_file artifacts_metadata artifacts_size created_at
        updated_at started_at finished_at queued_at erased_by
        erased_at auto_canceled_by job_artifacts job_artifacts_archive
-       job_artifacts_metadata job_artifacts_trace job_artifacts_junit].freeze
+       job_artifacts_metadata job_artifacts_trace job_artifacts_junit
+       job_artifacts_sast job_artifacts_dependency_scanning
+       job_artifacts_container_scanning job_artifacts_dast].freeze
 
   IGNORE_ACCESSORS =
     %i[type lock_version target_url base_tags trace_sections
@@ -38,9 +40,8 @@ describe Ci::RetryBuildService do
     let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }
 
     let(:build) do
-      create(:ci_build, :failed, :artifacts, :test_reports, :expired, :erased,
-             :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
-             :triggered, :trace_artifact, :teardown_environment,
+      create(:ci_build, :failed, :expired, :erased, :queued, :coverage, :tags,
+             :allowed_to_fail, :on_tag, :triggered, :teardown_environment,
              description: 'my-job', stage: 'test', stage_id: stage.id,
              pipeline: pipeline, auto_canceled_by: another_pipeline)
     end
@@ -50,6 +51,15 @@ describe Ci::RetryBuildService do
       # can reset one of the fields when assigning another. We plan to deprecate
       # and remove legacy `stage` column in the future.
       build.update(stage: 'test', stage_id: stage.id)
+
+      # Make sure we have one instance for every possible job_artifact_X
+      # associations to check they are correctly rejected on build duplication.
+      Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.each do |file_type, file_format|
+        create(:ci_job_artifact, file_format,
+               file_type: file_type, job: build, expire_at: build.artifacts_expire_at)
+      end
+
+      build.reload
     end
 
     describe 'clone accessors' do