Merge branch 'artifact-format-v2' into 'master'

Extend gitlab-ci.yml to request junit.xml test reports

See merge request gitlab-org/gitlab-ce!20390

2 files changed, 41 insertions, 9 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index db86400128c..35b20bc1e0b 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -22,9 +22,10 @@ module Ci
     has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id
 
     has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent
-    has_one :job_artifacts_archive, -> { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
-    has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
-    has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
+
+    Ci::JobArtifact.file_types.each do |key, value|
+      has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
+    end
 
     has_one :metadata, class_name: 'Ci::BuildMetadata'
     has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build
@@ -386,6 +387,10 @@ module Ci
       trace.exist?
     end
 
+    def has_test_reports?
+      job_artifacts.test_reports.any?
+    end
+
     def has_old_trace?
       old_trace.present?
     end
@@ -453,16 +458,22 @@ module Ci
       save
     end
 
+    def erase_test_reports!
+      # TODO: Use fast_destroy_all in the context of https://gitlab.com/gitlab-org/gitlab-ce/issues/35240
+      job_artifacts_junit&.destroy
+    end
+
     def erase(opts = {})
       return false unless erasable?
 
       erase_artifacts!
+      erase_test_reports!
       erase_trace!
       update_erased!(opts[:erased_by])
     end
 
     def erasable?
-      complete? && (artifacts? || has_trace?)
+      complete? && (artifacts? || has_test_reports? || has_trace?)
     end
 
     def erased?
@@ -539,10 +550,6 @@ module Ci
       Gitlab::Ci::Build::Image.from_services(self)
     end
 
-    def artifacts
-      [options[:artifacts]]
-    end
-
     def cache
       cache = options[:cache]
 
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index 3b952391b7e..054b714f8ac 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -4,11 +4,17 @@ module Ci
     include ObjectStorage::BackgroundMove
     extend Gitlab::Ci::Model
 
+    TEST_REPORT_FILE_TYPES = %w[junit].freeze
+    DEFAULT_FILE_NAMES = { junit: 'junit.xml' }.freeze
+    TYPE_AND_FORMAT_PAIRS = { archive: :zip, metadata: :gzip, trace: :raw, junit: :gzip }.freeze
+
     belongs_to :project
     belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id
 
     mount_uploader :file, JobArtifactUploader
 
+    validates :file_format, presence: true, unless: :trace?, on: :create
+    validate :valid_file_format?, unless: :trace?, on: :create
     before_save :set_size, if: :file_changed?
     after_save :update_project_statistics_after_save, if: :size_changed?
     after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed?
@@ -17,14 +23,33 @@ module Ci
 
     scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
 
+    scope :test_reports, -> do
+      types = self.file_types.select { |file_type| TEST_REPORT_FILE_TYPES.include?(file_type) }.values
+
+      where(file_type: types)
+    end
+
     delegate :exists?, :open, to: :file
 
     enum file_type: {
       archive: 1,
       metadata: 2,
-      trace: 3
+      trace: 3,
+      junit: 4
     }
 
+    enum file_format: {
+      raw: 1,
+      zip: 2,
+      gzip: 3
+    }
+
+    def valid_file_format?
+      unless TYPE_AND_FORMAT_PAIRS[self.file_type&.to_sym] == self.file_format&.to_sym
+        errors.add(:file_format, 'Invalid file format with specified file type')
+      end
+    end
+
     def update_file_store
       # The file.object_store is set during `uploader.store!`
       # which happens after object is inserted/updated