Merge branch 'mo-refactor-test-report-summary' into 'master'

Refactor TestReportSummary with total

See merge request gitlab-org/gitlab!37204

app/assets/javascripts/pipelines/stores/test_reports/mutations.js

@@ -10,7 +10,32 @@ export default {
   },
 
   [types.SET_SUMMARY](state, testReports) {
-    Object.assign(state, { testReports });
+    const { total } = testReports;
+    state.testReports = {
+      ...testReports,
+
+      /*
+        TLDR; this is a temporary mapping that will be updated once
+        test suites have the new data schema
+
+        The backend is in the middle of updating the data schema
+        to have a `total` object containing the total data values.
+        The test suites don't have the new schema, but the summary
+        does. Currently the `test_summary.vue` component takes both
+        the summary and a test suite depending on what is being viewed.
+        This is a temporary change to map the new schema to the old until
+        we can update the schema for the test suites also.
+        Since test suites is an array, it is easier to just map the summary
+        to the old schema instead of mapping every test suite to the new.
+      */
+
+      total_time: total.time,
+      total_count: total.count,
+      success_count: total.success,
+      failed_count: total.failed,
+      skipped_count: total.skipped,
+      error_count: total.error,
+    };
   },
 
   [types.TOGGLE_LOADING](state) {

app/serializers/pipeline_entity.rb

@@ -86,7 +86,7 @@ class PipelineEntity < Grape::Entity
   end
 
   expose :tests_total_count do |pipeline|
-    pipeline.test_report_summary.total_count
+    pipeline.test_report_summary.total[:count]
   end
 
   private

app/serializers/test_report_summary_entity.rb

 # frozen_string_literal: true
 
-class TestReportSummaryEntity < TestReportEntity
+class TestReportSummaryEntity < Grape::Entity
+  expose :total
+
   expose :test_suites, using: TestSuiteSummaryEntity do |summary|
     summary.test_suites.values
   end

app/views/projects/pipelines/_with_tabs.html.haml

@@ -23,7 +23,7 @@
       %li.js-tests-tab-link
         = link_to test_report_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-tests', action: 'test_report', toggle: 'tab' }, class: 'test-tab' do
           = s_('TestReports|Tests')
-          %span.badge.badge-pill.js-test-report-badge-counter= @pipeline.test_report_summary.total_count
+          %span.badge.badge-pill.js-test-report-badge-counter= @pipeline.test_report_summary.total[:count]
     = render_if_exists "projects/pipelines/tabs_holder", pipeline: @pipeline, project: @project
 
 .tab-content

lib/gitlab/ci/reports/test_report_summary.rb

@@ -4,42 +4,17 @@ module Gitlab
   module Ci
     module Reports
       class TestReportSummary
-        attr_reader :all_results
-
-        def initialize(all_results)
-          @all_results = all_results
+        def initialize(build_report_results)
+          @build_report_results = build_report_results
+          @suite_summary = TestSuiteSummary.new(@build_report_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
+          @suite_summary.to_h
         end
 
         def test_suites
-          all_results
+          @build_report_results
             .group_by(&:tests_name)
             .transform_values { |results| TestSuiteSummary.new(results) }
         end

lib/gitlab/ci/reports/test_suite_summary.rb

@@ -4,45 +4,54 @@ module Gitlab
   module Ci
     module Reports
       class TestSuiteSummary
-        attr_reader :results
-
-        def initialize(results)
-          @results = results
+        def initialize(build_report_results)
+          @build_report_results = build_report_results
         end
 
         def name
-          @name ||= results.first.tests_name
+          @name ||= @build_report_results.first.tests_name
         end
 
         # rubocop: disable CodeReuse/ActiveRecord
         def build_ids
-          results.pluck(:build_id)
+          @build_report_results.pluck(:build_id)
         end
 
         def total_time
-          @total_time ||= results.sum(&:tests_duration)
+          @total_time ||= @build_report_results.sum(&:tests_duration)
         end
 
         def success_count
-          @success_count ||= results.sum(&:tests_success)
+          @success_count ||= @build_report_results.sum(&:tests_success)
         end
 
         def failed_count
-          @failed_count ||= results.sum(&:tests_failed)
+          @failed_count ||= @build_report_results.sum(&:tests_failed)
         end
 
         def skipped_count
-          @skipped_count ||= results.sum(&:tests_skipped)
+          @skipped_count ||= @build_report_results.sum(&:tests_skipped)
         end
 
         def error_count
-          @error_count ||= results.sum(&:tests_errored)
+          @error_count ||= @build_report_results.sum(&:tests_errored)
         end
 
         def total_count
           @total_count ||= [success_count, failed_count, skipped_count, error_count].sum
         end
         # rubocop: disable CodeReuse/ActiveRecord
+
+        def to_h
+          {
+            time: total_time,
+            count: total_count,
+            success: success_count,
+            failed: failed_count,
+            skipped: skipped_count,
+            error: error_count
+          }
+        end
       end
     end
   end

spec/controllers/projects/pipelines/tests_controller_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe Projects::Pipelines::TestsController do
         get_tests_summary_json
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response['total_count']).to eq(2)
+        expect(json_response.dig('total', 'count')).to eq(2)
       end
     end
 
@@ -28,7 +28,7 @@ RSpec.describe Projects::Pipelines::TestsController do
         get_tests_summary_json
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response['total_count']).to eq(0)
+        expect(json_response.dig('total', 'count')).to eq(0)
       end
     end
   end

spec/features/projects/pipelines/pipeline_spec.rb

@@ -371,7 +371,7 @@ RSpec.describe 'Pipeline', :js do
 
       context 'with test reports' do
         it 'shows badge counter in Tests tab' do
-          expect(page.find('.js-test-report-badge-counter').text).to eq(pipeline.test_report_summary.total_count.to_s)
+          expect(page.find('.js-test-report-badge-counter').text).to eq(pipeline.test_report_summary.total[:count].to_s)
         end
 
         it 'calls summary.json endpoint', :js do

spec/frontend/pipelines/test_reports/stores/mutations_spec.js

@@ -44,10 +44,21 @@ describe('Mutations TestReports Store', () => {
 
   describe('set summary', () => {
     it('should set summary', () => {
-      const summary = { total_count: 1 };
+      const summary = {
+        total: { time: 0, count: 10, success: 1, failed: 2, skipped: 3, error: 4 },
+      };
+      const expectedSummary = {
+        ...summary,
+        total_time: 0,
+        total_count: 10,
+        success_count: 1,
+        failed_count: 2,
+        skipped_count: 3,
+        error_count: 4,
+      };
       mutations[types.SET_SUMMARY](mockState, summary);
 
-      expect(mockState.testReports).toEqual(summary);
+      expect(mockState.testReports).toEqual(expectedSummary);
     });
   });
 

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

@@ -11,68 +11,8 @@ RSpec.describe Gitlab::Ci::Reports::TestReportSummary 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)
+      it 'returns all the total count in a hash' do
+        expect(subject).to include(:time, :count, :success, :failed, :skipped, :error)
       end
     end
   end

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

@@ -86,4 +86,14 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteSummary do
       end
     end
   end
+
+  describe '#to_h' do
+    subject { test_suite_summary.to_h }
+
+    context 'when test suite summary has several build report results' do
+      it 'returns the total as a hash' do
+        expect(subject).to include(:time, :count, :success, :failed, :skipped, :error)
+      end
+    end
+  end
 end

spec/models/ci/pipeline_spec.rb

@@ -3069,24 +3069,14 @@ RSpec.describe Ci::Pipeline, :mailer do
         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)
+      it 'returns test report summary with collected data' do
+        expect(subject.total).to include(time: 0.84, count: 4, success: 0, failed: 0, skipped: 0, error: 4)
       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)
+      it 'returns empty test report summary' do
+        expect(subject.total).to include(time: 0, count: 0, success: 0, failed: 0, skipped: 0, error: 0)
       end
     end
   end

spec/serializers/test_report_summary_entity_spec.rb

@@ -9,12 +9,8 @@ RSpec.describe TestReportSummaryEntity do
   describe '#as_json' do
     subject(:as_json) { entity.as_json }
 
-    it 'contains the total time' do
-      expect(as_json).to include(:total_time)
-    end
-
-    it 'contains the counts' do
-      expect(as_json).to include(:total_count, :success_count, :failed_count, :skipped_count, :error_count)
+    it 'contains the total' do
+      expect(as_json).to include(:total)
     end
 
     context 'when summary has test suites' do

spec/serializers/test_suite_summary_entity_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe TestSuiteSummaryEntity do
   let(:pipeline) { create(:ci_pipeline, :with_report_results) }
-  let(:entity) { described_class.new(pipeline.test_report_summary.total) }
+  let(:entity) { described_class.new(pipeline.test_report_summary.test_suites.each_value.first) }
 
   describe '#as_json' do
     subject(:as_json) { entity.as_json }