Expose test report summary API

This MR exposes a new endpoint to improve the performance of our
junit feature. We are now able to read data from the database
instead of parsing and loading all reports in memory.

app/controllers/projects/pipelines/tests_controller.rb

+# frozen_string_literal: true
+
+module Projects
+  module Pipelines
+    class TestsController < Projects::ApplicationController
+      before_action :pipeline
+      before_action :authorize_read_pipeline!
+      before_action :authorize_read_build!
+      before_action :validate_feature_flag!
+
+      def summary
+        respond_to do |format|
+          format.json do
+            render json: TestReportSerializer
+              .new(project: project, current_user: @current_user)
+              .represent(pipeline.test_report_summary)
+          end
+        end
+      end
+
+      private
+
+      def validate_feature_flag!
+        render_404 unless Feature.enabled?(:build_report_summary, project)
+      end
+
+      def pipeline
+        project.all_pipelines.find(tests_params[:id])
+      end
+
+      def tests_params
+        params.permit(:id)
+      end
+    end
+  end
+end

app/controllers/projects/pipelines_controller.rb

@@ -186,7 +186,7 @@ class Projects::PipelinesController < Projects::ApplicationController
       format.json do
         render json: TestReportSerializer
           .new(current_user: @current_user)
-          .represent(pipeline_test_report, project: project)
+          .represent(pipeline_test_report, project: project, details: true)
       end
     end
   end

app/models/ci/pipeline.rb

@@ -80,6 +80,7 @@ module Ci
     has_one :pipeline_config, class_name: 'Ci::PipelineConfig', inverse_of: :pipeline
 
     has_many :daily_build_group_report_results, class_name: 'Ci::DailyBuildGroupReportResult', foreign_key: :last_pipeline_id
+    has_many :latest_builds_report_results, through: :latest_builds, source: :report_results
 
     accepts_nested_attributes_for :variables, reject_if: :persisted?
 
@@ -802,6 +803,10 @@ module Ci
       complete? && latest_report_builds(reports_scope).exists?
     end
 
+    def test_report_summary
+      Gitlab::Ci::Reports::TestReportSummary.new(latest_builds_report_results)
+    end
+
     def test_reports
       Gitlab::Ci::Reports::TestReports.new.tap do |test_reports|
         latest_report_builds(Ci::JobArtifact.test_reports).preload(:project).find_each do |build|

app/serializers/test_suite_entity.rb

@@ -9,9 +9,11 @@ class TestSuiteEntity < Grape::Entity
   expose :failed_count
   expose :skipped_count
   expose :error_count
-  expose :suite_error
 
-  expose :test_cases, using: TestCaseEntity do |test_suite|
-    test_suite.suite_error ? [] : test_suite.test_cases.values.flat_map(&:values)
+  with_options if: -> (_, opts) { opts[:details] } do |test_suite|
+    expose :suite_error
+    expose :test_cases, using: TestCaseEntity do |test_suite|
+      test_suite.suite_error ? [] : test_suite.test_cases.values.flat_map(&:values)
+    end
   end
 end

config/routes/pipelines.rb

@@ -26,6 +26,12 @@ resources :pipelines, only: [:index, :new, :create, :show, :destroy] do
     resources :stages, only: [], param: :name do
       post :play_manual
     end
+
+    resources :tests, only: [], controller: 'pipelines/tests' do
+      collection do
+        get :summary
+      end
+    end
   end
 end
 

lib/api/pipelines.rb

@@ -120,7 +120,7 @@ module API
 
         authorize! :read_build, pipeline
 
-        present pipeline.test_reports, with: TestReportEntity
+        present pipeline.test_reports, with: TestReportEntity, details: true
       end
 
       desc 'Deletes a pipeline' do

lib/gitlab/ci/reports/test_report_summary.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Reports
+      class TestReportSummary
+        attr_reader :all_results
+
+        def initialize(all_results)
+          @all_results = all_results
+        end
+
+        def total
+          TestSuiteSummary.new(all_results)
+        end
+
+        def total_time
+          total.total_time
+        end
+
+        def total_count
+          total.total_count
+        end
+
+        def success_count
+          total.success_count
+        end
+
+        def failed_count
+          total.failed_count
+        end
+
+        def skipped_count
+          total.skipped_count
+        end
+
+        def error_count
+          total.error_count
+        end
+
+        def test_suites
+          all_results
+            .group_by(&:tests_name)
+            .transform_values { |results| TestSuiteSummary.new(results) }
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/reports/test_suite_summary.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Reports
+      class TestSuiteSummary
+        attr_reader :results
+
+        def initialize(results)
+          @results = results
+        end
+
+        def name
+          @name ||= results.first.tests_name
+        end
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def total_time
+          @total_time ||= results.sum(&:tests_duration)
+        end
+
+        def success_count
+          @success_count ||= results.sum(&:tests_success)
+        end
+
+        def failed_count
+          @failed_count ||= results.sum(&:tests_failed)
+        end
+
+        def skipped_count
+          @skipped_count ||= results.sum(&:tests_skipped)
+        end
+
+        def error_count
+          @error_count ||= results.sum(&:tests_errored)
+        end
+
+        def total_count
+          @total_count ||= [success_count, failed_count, skipped_count, error_count].sum
+        end
+        # rubocop: disable CodeReuse/ActiveRecord
+      end
+    end
+  end
+end

spec/controllers/projects/pipelines/tests_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Pipelines::TestsController do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :public, :repository) }
+  let(:pipeline) { create(:ci_pipeline, project: project) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'GET #summary.json' do
+    context 'when pipeline has build report results' do
+      let(:pipeline) { create(:ci_pipeline, :with_report_results, project: project) }
+
+      it 'renders test report summary data' do
+        get_tests_summary_json
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['total_count']).to eq(2)
+      end
+    end
+
+    context 'when pipeline does not have build report results' do
+      it 'renders test report summary data' do
+        get_tests_summary_json
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['total_count']).to eq(0)
+      end
+    end
+
+    context 'when feature is disabled' do
+      before do
+        stub_feature_flags(build_report_summary: false)
+      end
+
+      it 'returns 404' do
+        get_tests_summary_json
+
+        expect(response).to have_gitlab_http_status(:not_found)
+        expect(response.body).to be_empty
+      end
+    end
+  end
+
+  def get_tests_summary_json
+    get :summary,
+      params: {
+        namespace_id: project.namespace,
+        project_id: project,
+        id: pipeline.id
+      },
+      format: :json
+  end
+end

spec/factories/ci/builds.rb

@@ -302,6 +302,12 @@ FactoryBot.define do
       end
     end
 
+    trait :report_results do
+      after(:build) do |build|
+        build.report_results << build(:ci_build_report_result)
+      end
+    end
+
     trait :test_reports do
       after(:build) do |build|
         build.job_artifacts << create(:ci_job_artifact, :junit, job: build)

spec/factories/ci/pipelines.rb

@@ -65,6 +65,14 @@ FactoryBot.define do
         add_attribute(:protected) { true }
       end
 
+      trait :with_report_results do
+        status { :success }
+
+        after(:build) do |pipeline, evaluator|
+          pipeline.builds << build(:ci_build, :report_results, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
       trait :with_test_reports do
         status { :success }
 

spec/lib/gitlab/ci/reports/test_report_summary_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Reports::TestReportSummary do
+  let(:build_report_result_1) { build(:ci_build_report_result) }
+  let(:build_report_result_2) { build(:ci_build_report_result, :with_junit_success) }
+  let(:test_report_summary) { described_class.new([build_report_result_1, build_report_result_2]) }
+
+  describe '#total' do
+    subject { test_report_summary.total }
+
+    context 'when test report summary has several build report results' do
+      it 'returns test suite summary object' do
+        expect(subject).to be_a_kind_of(Gitlab::Ci::Reports::TestSuiteSummary)
+      end
+    end
+  end
+
+  describe '#total_time' do
+    subject { test_report_summary.total_time }
+
+    context 'when test report summary has several build report results' do
+      it 'returns the total' do
+        expect(subject).to eq(0.84)
+      end
+    end
+  end
+
+  describe '#total_count' do
+    subject { test_report_summary.total_count }
+
+    context 'when test report summary has several build report results' do
+      it 'returns the total count' do
+        expect(subject).to eq(4)
+      end
+    end
+  end
+
+  describe '#success_count' do
+    subject { test_report_summary.success_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total success' do
+        expect(subject).to eq(2)
+      end
+    end
+  end
+
+  describe '#failed_count' do
+    subject { test_report_summary.failed_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total failed' do
+        expect(subject).to eq(0)
+      end
+    end
+  end
+
+  describe '#error_count' do
+    subject { test_report_summary.error_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total errored' do
+        expect(subject).to eq(2)
+      end
+    end
+  end
+
+  describe '#skipped_count' do
+    subject { test_report_summary.skipped_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total skipped' do
+        expect(subject).to eq(0)
+      end
+    end
+  end
+
+  describe '#test_suites' do
+    subject { test_report_summary.test_suites }
+
+    context 'when test report summary has several build report results' do
+      it 'returns test suites grouped by name' do
+        expect(subject.keys).to eq(["rspec"])
+        expect(subject.keys.size).to eq(1)
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/reports/test_suite_summary_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Reports::TestSuiteSummary do
+  let(:build_report_result_1) { build(:ci_build_report_result) }
+  let(:build_report_result_2) { build(:ci_build_report_result, :with_junit_success) }
+  let(:test_suite_summary) { described_class.new([build_report_result_1, build_report_result_2]) }
+
+  describe '#name' do
+    subject { test_suite_summary.name }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the suite name' do
+        expect(subject).to eq("rspec")
+      end
+    end
+  end
+
+  describe '#total_time' do
+    subject { test_suite_summary.total_time }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total time' do
+        expect(subject).to eq(0.84)
+      end
+    end
+  end
+
+  describe '#success_count' do
+    subject { test_suite_summary.success_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total success' do
+        expect(subject).to eq(2)
+      end
+    end
+  end
+
+  describe '#failed_count' do
+    subject { test_suite_summary.failed_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total failed' do
+        expect(subject).to eq(0)
+      end
+    end
+  end
+
+  describe '#error_count' do
+    subject { test_suite_summary.error_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total errored' do
+        expect(subject).to eq(2)
+      end
+    end
+  end
+
+  describe '#skipped_count' do
+    subject { test_suite_summary.skipped_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total skipped' do
+        expect(subject).to eq(0)
+      end
+    end
+  end
+
+  describe '#total_count' do
+    subject { test_suite_summary.total_count }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total count' do
+        expect(subject).to eq(4)
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -226,6 +226,7 @@ ci_pipelines:
 - daily_build_group_report_results
 - latest_builds
 - daily_report_results
+- latest_builds_report_results
 ci_refs:
 - project
 - ci_pipelines

spec/models/ci/pipeline_spec.rb

@@ -2891,6 +2891,39 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#test_report_summary' do
+    subject { pipeline.test_report_summary }
+
+    context 'when pipeline has multiple builds with report results' do
+      let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+      before do
+        create(:ci_build, :success, :report_results, name: 'rspec', pipeline: pipeline, project: project)
+        create(:ci_build, :success, :report_results, name: 'java', pipeline: pipeline, project: project)
+      end
+
+      it 'returns test report summary with collected data', :aggregate_failures do
+        expect(subject.total_time).to be(0.84)
+        expect(subject.total_count).to be(4)
+        expect(subject.success_count).to be(0)
+        expect(subject.failed_count).to be(0)
+        expect(subject.error_count).to be(4)
+        expect(subject.skipped_count).to be(0)
+      end
+    end
+
+    context 'when pipeline does not have any builds with report results' do
+      it 'returns empty test report sumary', :aggregate_failures do
+        expect(subject.total_time).to be(0)
+        expect(subject.total_count).to be(0)
+        expect(subject.success_count).to be(0)
+        expect(subject.failed_count).to be(0)
+        expect(subject.error_count).to be(0)
+        expect(subject.skipped_count).to be(0)
+      end
+    end
+  end
+
   describe '#test_reports' do
     subject { pipeline.test_reports }
 

spec/serializers/test_suite_entity_spec.rb

@@ -2,36 +2,46 @@
 
 require 'spec_helper'
 
-describe TestSuiteEntity do
-  let(:pipeline)   { create(:ci_pipeline, :with_test_reports) }
+RSpec.describe TestSuiteEntity do
+  let(:pipeline) { create(:ci_pipeline, :with_test_reports) }
   let(:test_suite) { pipeline.test_reports.test_suites.each_value.first }
-  let(:entity)     { described_class.new(test_suite) }
+  let(:user) { create(:user) }
+  let(:request) { double('request', current_user: user) }
 
-  describe '#as_json' do
-    subject(:as_json) { entity.as_json }
+  subject { described_class.new(test_suite, request: request).as_json }
+
+  context 'when details option is not present' do
+    it 'does not expose suite error and test cases', :aggregate_failures do
+      expect(subject).not_to include(:test_cases)
+      expect(subject).not_to include(:suite_error)
+    end
+  end
+
+  context 'when details option is present' do
+    subject { described_class.new(test_suite, request: request, details: true).as_json }
 
     it 'contains the suite name' do
-      expect(as_json[:name]).to be_present
+      expect(subject[:name]).to be_present
     end
 
     it 'contains the total time' do
-      expect(as_json[:total_time]).to be_present
+      expect(subject[:total_time]).to be_present
     end
 
     it 'contains the counts' do
-      expect(as_json[:total_count]).to eq(4)
-      expect(as_json[:success_count]).to eq(2)
-      expect(as_json[:failed_count]).to eq(2)
-      expect(as_json[:skipped_count]).to eq(0)
-      expect(as_json[:error_count]).to eq(0)
+      expect(subject[:total_count]).to eq(4)
+      expect(subject[:success_count]).to eq(2)
+      expect(subject[:failed_count]).to eq(2)
+      expect(subject[:skipped_count]).to eq(0)
+      expect(subject[:error_count]).to eq(0)
     end
 
     it 'contains the test cases' do
-      expect(as_json[:test_cases].count).to eq(4)
+      expect(subject[:test_cases].count).to eq(4)
     end
 
     it 'contains an empty error message' do
-      expect(as_json[:suite_error]).to be_nil
+      expect(subject[:suite_error]).to be_nil
     end
 
     context 'with a suite error' do
@@ -40,27 +50,27 @@ describe TestSuiteEntity do
       end
 
       it 'contains the suite name' do
-        expect(as_json[:name]).to be_present
+        expect(subject[:name]).to be_present
       end
 
       it 'contains the total time' do
-        expect(as_json[:total_time]).to be_present
+        expect(subject[:total_time]).to be_present
       end
 
       it 'returns all the counts as 0' do
-        expect(as_json[:total_count]).to eq(0)
-        expect(as_json[:success_count]).to eq(0)
-        expect(as_json[:failed_count]).to eq(0)
-        expect(as_json[:skipped_count]).to eq(0)
-        expect(as_json[:error_count]).to eq(0)
+        expect(subject[:total_count]).to eq(0)
+        expect(subject[:success_count]).to eq(0)
+        expect(subject[:failed_count]).to eq(0)
+        expect(subject[:skipped_count]).to eq(0)
+        expect(subject[:error_count]).to eq(0)
       end
 
       it 'returns no test cases' do
-        expect(as_json[:test_cases]).to be_empty
+        expect(subject[:test_cases]).to be_empty
       end
 
       it 'returns a suite error' do
-        expect(as_json[:suite_error]).to eq('a really bad error')
+        expect(subject[:suite_error]).to eq('a really bad error')
       end
     end
   end