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

Parse junit.xml.gz and calculate the difference between head and base

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

app/controllers/projects/merge_requests_controller.rb

@@ -99,6 +99,23 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     }
   end
 
+  def test_reports
+    result = @merge_request.compare_test_reports
+
+    Gitlab::PollingInterval.set_header(response, interval: 10_000)
+
+    case result[:status]
+    when :parsing
+      render json: '', status: :no_content
+    when :parsed
+      render json: result[:data].to_json, status: :ok
+    when :error
+      render json: { status_reason: result[:status_reason] }, status: :bad_request
+    else
+      render json: { status_reason: 'Unknown error' }, status: :internal_server_error
+    end
+  end
+
   def edit
     define_edit_vars
   end

app/models/ci/build.rb

@@ -71,6 +71,11 @@ module Ci
       where('NOT EXISTS (?)', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').trace)
     end
 
+    scope :with_test_reports, ->() do
+      includes(:job_artifacts_junit) # Prevent N+1 problem when iterating each ci_job_artifact row
+        .where('EXISTS (?)', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').test_reports)
+    end
+
     scope :with_artifacts_stored_locally, -> { with_artifacts_archive.where(artifacts_file_store: [nil, LegacyArtifactUploader::Store::LOCAL]) }
     scope :with_artifacts_not_expired, ->() { with_artifacts_archive.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts_archive.where('artifacts_expire_at < ?', Time.now) }
@@ -629,8 +634,24 @@ module Ci
       running? && runner_session_url.present?
     end
 
+    def collect_test_reports!(test_reports)
+      test_reports.get_suite(group_name).tap do |test_suite|
+        each_test_report do |file_type, blob|
+          Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite)
+        end
+      end
+    end
+
     private
 
+    def each_test_report
+      Ci::JobArtifact::TEST_REPORT_FILE_TYPES.each do |file_type|
+        public_send("job_artifacts_#{file_type}").each_blob do |blob| # rubocop:disable GitlabSecurity/PublicSend
+          yield file_type, blob
+        end
+      end
+    end
+
     def update_artifacts_size
       self.artifacts_size = legacy_artifacts_file&.size
     end

app/models/ci/job_artifact.rb

@@ -6,6 +6,8 @@ module Ci
     include ObjectStorage::BackgroundMove
     extend Gitlab::Ci::Model
 
+    NotSupportedAdapterError = Class.new(StandardError)
+
     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
@@ -46,6 +48,10 @@ module Ci
       gzip: 3
     }
 
+    FILE_FORMAT_ADAPTERS = {
+      gzip: Gitlab::Ci::Build::Artifacts::GzipFileAdapter
+    }.freeze
+
     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')
@@ -77,8 +83,22 @@ module Ci
         end
     end
 
+    def each_blob(&blk)
+      unless file_format_adapter_class
+        raise NotSupportedAdapterError, 'This file format requires a dedicated adapter'
+      end
+
+      file.open do |stream|
+        file_format_adapter_class.new(stream).each_blob(&blk)
+      end
+    end
+
     private
 
+    def file_format_adapter_class
+      FILE_FORMAT_ADAPTERS[file_format.to_sym]
+    end
+
     def set_size
       self.size = file.size
     end

app/models/ci/pipeline.rb

@@ -605,6 +605,18 @@ module Ci
       @latest_builds_with_artifacts ||= builds.latest.with_artifacts_archive.to_a
     end
 
+    def has_test_reports?
+      complete? && builds.with_test_reports.any?
+    end
+
+    def test_reports
+      Gitlab::Ci::Reports::TestReports.new.tap do |test_reports|
+        builds.with_test_reports.each do |build|
+          build.collect_test_reports!(test_reports)
+        end
+      end
+    end
+
     private
 
     def ci_yaml_from_repo

app/models/merge_request.rb

@@ -13,6 +13,11 @@ class MergeRequest < ActiveRecord::Base
   include ThrottledTouch
   include Gitlab::Utils::StrongMemoize
   include LabelEventable
+  include ReactiveCaching
+
+  self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
+  self.reactive_cache_refresh_interval = 1.hour
+  self.reactive_cache_lifetime = 1.hour
 
   ignore_column :locked_at,
                 :ref_fetched,
@@ -1012,6 +1017,30 @@ class MergeRequest < ActiveRecord::Base
       .order(id: :desc)
   end
 
+  def has_test_reports?
+    actual_head_pipeline&.has_test_reports?
+  end
+
+  def compare_test_reports
+    unless has_test_reports?
+      return { status: :error, status_reason: 'This merge request does not have test reports' }
+    end
+
+    with_reactive_cache(
+      :compare_test_results,
+      base_pipeline&.iid,
+      actual_head_pipeline.iid) { |data| data } || { status: :parsing }
+  end
+
+  def calculate_reactive_cache(identifier, *args)
+    case identifier.to_sym
+    when :compare_test_results
+      Ci::CompareTestReportsService.new(project).execute(*args)
+    else
+      raise NotImplementedError, "Unknown identifier: #{identifier}"
+    end
+  end
+
   def all_commits
     # MySQL doesn't support LIMIT in a subquery.
     diffs_relation = if Gitlab::Database.postgresql?
@@ -1124,6 +1153,12 @@ class MergeRequest < ActiveRecord::Base
     true
   end
 
+  def base_pipeline
+    @base_pipeline ||= project.pipelines
+      .order(id: :desc)
+      .find_by(sha: diff_base_sha)
+  end
+
   def discussions_rendered_on_frontend?
     true
   end

app/serializers/merge_request_widget_entity.rb

@@ -231,6 +231,12 @@ class MergeRequestWidgetEntity < IssuableEntity
     end
   end
 
+  expose :test_reports_path do |merge_request|
+    if merge_request.has_test_reports?
+      test_reports_project_merge_request_path(merge_request.project, merge_request, format: :json)
+    end
+  end
+
   private
 
   delegate :current_user, to: :request

app/serializers/test_case_entity.rb

+class TestCaseEntity < Grape::Entity
+  expose :status
+  expose :name
+  expose :execution_time
+  expose :system_output
+  expose :stack_trace
+end

app/serializers/test_reports_comparer_entity.rb

+class TestReportsComparerEntity < Grape::Entity
+  expose :total_status, as: :status
+
+  expose :summary do
+    expose :total_count, as: :total
+    expose :resolved_count, as: :resolved
+    expose :failed_count, as: :failed
+  end
+
+  expose :suite_comparers, as: :suites, using: TestSuiteComparerEntity
+end

app/serializers/test_reports_comparer_serializer.rb

+class TestReportsComparerSerializer < BaseSerializer
+  entity TestReportsComparerEntity
+end

app/serializers/test_suite_comparer_entity.rb

+class TestSuiteComparerEntity < Grape::Entity
+  expose :name
+  expose :total_status, as: :status
+
+  expose :summary do
+    expose :total_count, as: :total
+    expose :resolved_count, as: :resolved
+    expose :failed_count, as: :failed
+  end
+
+  expose :new_failures, using: TestCaseEntity
+  expose :resolved_failures, using: TestCaseEntity
+  expose :existing_failures, using: TestCaseEntity
+end

app/services/ci/compare_test_reports_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class CompareTestReportsService < ::BaseService
+    def execute(base_pipeline_iid, head_pipeline_iid)
+      base_pipeline = project.pipelines.find_by_iid(base_pipeline_iid) if base_pipeline_iid
+      head_pipeline = project.pipelines.find_by_iid(head_pipeline_iid)
+
+      begin
+        comparer = Gitlab::Ci::Reports::TestReportsComparer
+          .new(base_pipeline&.test_reports, head_pipeline.test_reports)
+
+        {
+          status: :parsed,
+          data: TestReportsComparerSerializer
+            .new(project: project)
+            .represent(comparer).as_json
+        }
+      rescue => e
+        { status: :error, status_reason: e.message }
+      end
+    end
+  end
+end

changelogs/unreleased/artifact-format-v2-with-parser.yml

+---
+title: JUnit XML Test Summary In MR widget
+merge_request: 20576
+author:
+type: added

config/routes/project.rb

@@ -109,6 +109,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           post :assign_related_issues
           get :discussions, format: :json
           post :rebase
+          get :test_reports
 
           scope constraints: { format: nil }, action: :show do
             get :commits, defaults: { tab: 'commits' }

lib/gitlab/ci/build/artifacts/gzip_file_adapter.rb

+module Gitlab
+  module Ci
+    module Build
+      module Artifacts
+        class GzipFileAdapter
+          attr_reader :stream
+
+          InvalidStreamError = Class.new(StandardError)
+
+          def initialize(stream)
+            raise InvalidStreamError, "Stream is required" unless stream
+
+            @stream = stream
+          end
+
+          def each_blob
+            stream.seek(0)
+
+            until stream.eof?
+              gzip(stream) do |gz|
+                yield gz.read, gz.orig_name
+                unused = gz.unused&.length.to_i
+                # pos has already reached to EOF at the moment
+                # We rewind the pos to the top of unused files
+                # to read next gzip stream, to support multistream archives
+                # https://golang.org/src/compress/gzip/gunzip.go#L117
+                stream.seek(-unused, IO::SEEK_CUR)
+              end
+            end
+          end
+
+          private
+
+          def gzip(stream, &block)
+            gz = Zlib::GzipReader.new(stream)
+            yield(gz)
+          rescue Zlib::Error => e
+            raise InvalidStreamError, e.message
+          ensure
+            gz&.finish
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/parsers.rb

+module Gitlab
+  module Ci
+    module Parsers
+      def self.fabricate!(file_type)
+        "Gitlab::Ci::Parsers::#{file_type.classify}".constantize.new
+      end
+    end
+  end
+end

lib/gitlab/ci/parsers/junit.rb

+module Gitlab
+  module Ci
+    module Parsers
+      class Junit
+        attr_reader :data
+
+        JunitParserError = Class.new(StandardError)
+
+        def parse!(xml_data, test_suite)
+          @data = Hash.from_xml(xml_data)
+
+          each_suite do |testcases|
+            testcases.each do |testcase|
+              test_case = create_test_case(testcase)
+              test_suite.add_test_case(test_case)
+            end
+          end
+        rescue REXML::ParseException => e
+          raise JunitParserError, "XML parsing failed: #{e.message}"
+        rescue => e
+          raise JunitParserError, "JUnit parsing failed: #{e.message}"
+        end
+
+        private
+
+        def each_suite
+          testsuites.each do |testsuite|
+            yield testcases(testsuite)
+          end
+        end
+
+        def testsuites
+          if data['testsuites']
+            data['testsuites']['testsuite']
+          else
+            [data['testsuite']]
+          end
+        end
+
+        def testcases(testsuite)
+          if testsuite['testcase'].is_a?(Array)
+            testsuite['testcase']
+          else
+            [testsuite['testcase']]
+          end
+        end
+
+        def create_test_case(data)
+          if data['failure']
+            status = ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED
+            system_output = data['failure']
+          else
+            status = ::Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS
+            system_output = nil
+          end
+
+          ::Gitlab::Ci::Reports::TestCase.new(
+            classname: data['classname'],
+            name: data['name'],
+            file: data['file'],
+            execution_time: data['time'],
+            status: status,
+            system_output: system_output
+          )
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/reports/test_case.rb

+module Gitlab
+  module Ci
+    module Reports
+      class TestCase
+        STATUS_SUCCESS = 'success'.freeze
+        STATUS_FAILED = 'failed'.freeze
+        STATUS_SKIPPED = 'skipped'.freeze
+        STATUS_ERROR = 'error'.freeze
+        STATUS_TYPES = [STATUS_SUCCESS, STATUS_FAILED, STATUS_SKIPPED, STATUS_ERROR].freeze
+
+        attr_reader :name, :classname, :execution_time, :status, :file, :system_output, :stack_trace, :key
+
+        def initialize(name:, classname:, execution_time:, status:, file: nil, system_output: nil, stack_trace: nil)
+          @name = name
+          @classname = classname
+          @file = file
+          @execution_time = execution_time.to_f
+          @status = status
+          @system_output = system_output
+          @stack_trace = stack_trace
+          @key = sanitize_key_name("#{classname}_#{name}")
+        end
+
+        private
+
+        def sanitize_key_name(key)
+          key.gsub(/[^0-9A-Za-z]/, '-')
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/reports/test_reports.rb

+module Gitlab
+  module Ci
+    module Reports
+      class TestReports
+        attr_reader :test_suites
+
+        def initialize
+          @test_suites = {}
+        end
+
+        def get_suite(suite_name)
+          test_suites[suite_name] ||= TestSuite.new(suite_name)
+        end
+
+        def total_time
+          test_suites.values.sum(&:total_time)
+        end
+
+        def total_count
+          test_suites.values.sum(&:total_count)
+        end
+
+        def total_status
+          if failed_count > 0 || error_count > 0
+            TestCase::STATUS_FAILED
+          else
+            TestCase::STATUS_SUCCESS
+          end
+        end
+
+        TestCase::STATUS_TYPES.each do |status_type|
+          define_method("#{status_type}_count") do
+            test_suites.values.sum { |suite| suite.public_send("#{status_type}_count") } # rubocop:disable GitlabSecurity/PublicSend
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/reports/test_reports_comparer.rb

+module Gitlab
+  module Ci
+    module Reports
+      class TestReportsComparer
+        include Gitlab::Utils::StrongMemoize
+
+        attr_reader :base_reports, :head_reports
+
+        def initialize(base_reports, head_reports)
+          @base_reports = base_reports || TestReports.new
+          @head_reports = head_reports
+        end
+
+        def suite_comparers
+          strong_memoize(:suite_comparers) do
+            head_reports.test_suites.map do |name, test_suite|
+              TestSuiteComparer.new(name, base_reports.get_suite(name), test_suite)
+            end
+          end
+        end
+
+        def total_status
+          if suite_comparers.any? { |suite| suite.total_status == TestCase::STATUS_FAILED }
+            TestCase::STATUS_FAILED
+          else
+            TestCase::STATUS_SUCCESS
+          end
+        end
+
+        %w(total_count resolved_count failed_count).each do |method|
+          define_method(method) do
+            suite_comparers.sum { |suite| suite.public_send(method) } # rubocop:disable GitlabSecurity/PublicSend
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/reports/test_suite.rb

+module Gitlab
+  module Ci
+    module Reports
+      class TestSuite
+        attr_reader :name
+        attr_reader :test_cases
+        attr_reader :total_time
+
+        def initialize(name = nil)
+          @name = name
+          @test_cases = {}
+          @total_time = 0.0
+          @duplicate_cases = []
+        end
+
+        def add_test_case(test_case)
+          @duplicate_cases << test_case if existing_key?(test_case)
+
+          @test_cases[test_case.status] ||= {}
+          @test_cases[test_case.status][test_case.key] = test_case
+          @total_time += test_case.execution_time
+        end
+
+        def total_count
+          test_cases.values.sum(&:count)
+        end
+
+        def total_status
+          if failed_count > 0 || error_count > 0
+            TestCase::STATUS_FAILED
+          else
+            TestCase::STATUS_SUCCESS
+          end
+        end
+
+        TestCase::STATUS_TYPES.each do |status_type|
+          define_method("#{status_type}") do
+            test_cases[status_type] || {}
+          end
+
+          define_method("#{status_type}_count") do
+            test_cases[status_type]&.length.to_i
+          end
+        end
+
+        private
+
+        def existing_key?(test_case)
+          @test_cases[test_case.status]&.key?(test_case.key)
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/reports/test_suite_comparer.rb

+module Gitlab
+  module Ci
+    module Reports
+      class TestSuiteComparer
+        include Gitlab::Utils::StrongMemoize
+
+        attr_reader :name, :base_suite, :head_suite
+
+        def initialize(name, base_suite, head_suite)
+          @name = name
+          @base_suite = base_suite || TestSuite.new
+          @head_suite = head_suite
+        end
+
+        def new_failures
+          strong_memoize(:new_failures) do
+            head_suite.failed.reject do |key, _|
+              base_suite.failed.include?(key)
+            end.values
+          end
+        end
+
+        def existing_failures
+          strong_memoize(:existing_failures) do
+            head_suite.failed.select do |key, _|
+              base_suite.failed.include?(key)
+            end.values
+          end
+        end
+
+        def resolved_failures
+          strong_memoize(:resolved_failures) do
+            head_suite.success.select do |key, _|
+              base_suite.failed.include?(key)
+            end.values
+          end
+        end
+
+        def total_count
+          head_suite.total_count
+        end
+
+        def total_status
+          head_suite.total_status
+        end
+
+        def resolved_count
+          resolved_failures.count
+        end
+
+        def failed_count
+          new_failures.count + existing_failures.count
+        end
+      end
+    end
+  end
+end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -580,6 +580,64 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'GET test_reports' do
+    subject do
+      get :test_reports,
+          namespace_id: project.namespace.to_param,
+          project_id: project,
+          id: merge_request.iid,
+          format: :json
+    end
+
+    before do
+      allow_any_instance_of(MergeRequest)
+        .to receive(:compare_test_reports).and_return(comparison_status)