Merge branch 'eb-test-summary-suite-error' into 'master'

Include suite errors in test summary

See merge request gitlab-org/gitlab!55770

app/models/ci/build_report_result.rb

@@ -38,6 +38,10 @@ module Ci
       tests.dig("skipped").to_i
     end
 
+    def suite_error
+      tests.dig("suite_error")
+    end
+
     def tests_total
       [tests_success, tests_failed, tests_errored, tests_skipped].sum
     end

app/serializers/test_suite_summary_entity.rb

@@ -4,4 +4,6 @@ class TestSuiteSummaryEntity < TestSuiteEntity
   expose :build_ids do |summary|
     summary.build_ids
   end
+
+  expose :suite_error
 end

app/services/ci/build_report_result_service.rb

@@ -33,7 +33,8 @@ module Ci
           failed: test_suite.failed_count,
           errored: test_suite.error_count,
           skipped: test_suite.skipped_count,
-          success: test_suite.success_count
+          success: test_suite.success_count,
+          suite_error: test_suite.suite_error
         }
       }
     end

changelogs/unreleased/eb-test-summary-suite-error.yml

+---
+title: Display suite errors on test summary page
+merge_request: 55770
+author:
+type: changed

doc/ci/unit_test_reports.md

@@ -316,13 +316,21 @@ If JUnit report format XML files are generated and uploaded as part of a pipelin
 can be viewed inside the pipelines details page. The **Tests** tab on this page will
 display a list of test suites and cases reported from the XML file.
 
-![Test Reports Widget](img/pipelines_junit_test_report_ui_v12_5.png)
+![Test Reports Widget](img/pipelines_junit_test_report_v13_10.png)
 
 You can view all the known test suites and click on each of these to see further
 details, including the cases that make up the suite.
 
 You can also retrieve the reports via the [GitLab API](../api/pipelines.md#get-a-pipelines-test-report).
 
+### Unit test reports parsing errors
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/263457) in GitLab 13.10.
+
+If parsing JUnit report XML results in an error, an indicator is shown next to the job name. Hovering over the icon shows the parser error in a tooltip. If multiple parsing errors come from [grouped jobs](jobs/index.md#group-jobs-in-a-pipeline), GitLab shows only the first error from the group.
+
+![Test Reports With Errors](img/pipelines_junit_test_report_with_errors_v13_10.png)
+
 ## Viewing JUnit screenshots on GitLab
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/202114) in GitLab 13.0.

lib/gitlab/ci/reports/test_suite_summary.rb

@@ -4,6 +4,8 @@ module Gitlab
   module Ci
     module Reports
       class TestSuiteSummary
+        include Gitlab::Utils::StrongMemoize
+
         def initialize(build_report_results)
           @build_report_results = build_report_results
         end
@@ -42,6 +44,12 @@ module Gitlab
         end
         # rubocop: disable CodeReuse/ActiveRecord
 
+        def suite_error
+          strong_memoize(:suite_error) do
+            @build_report_results.map(&:suite_error).compact.first
+          end
+        end
+
         def to_h
           {
             time: total_time,
@@ -49,7 +57,8 @@ module Gitlab
             success: success_count,
             failed: failed_count,
             skipped: skipped_count,
-            error: error_count
+            error: error_count,
+            suite_error: suite_error
           }
         end
       end

spec/factories/ci/build_report_results.rb

@@ -4,10 +4,15 @@ FactoryBot.define do
   factory :ci_build_report_result, class: 'Ci::BuildReportResult' do
     build factory: :ci_build
     project factory: :project
+
+    transient do
+      test_suite_name { "rspec" }
+    end
+
     data do
       {
         tests: {
-          name: "rspec",
+          name: test_suite_name,
           duration: 0.42,
           failed: 0,
           errored: 2,
@@ -21,7 +26,7 @@ FactoryBot.define do
       data do
         {
           tests: {
-            name: "rspec",
+            name: test_suite_name,
             duration: 0.42,
             failed: 0,
             errored: 0,
@@ -31,5 +36,25 @@ FactoryBot.define do
         }
       end
     end
+
+    trait :with_junit_suite_error do
+      transient do
+        test_suite_error { "some error" }
+      end
+
+      data do
+        {
+          tests: {
+            name: test_suite_name,
+            duration: 0.42,
+            failed: 0,
+            errored: 0,
+            skipped: 0,
+            success: 2,
+            suite_error: test_suite_error
+          }
+        }
+      end
+    end
   end
 end

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

@@ -87,12 +87,44 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteSummary do
     end
   end
 
+  describe '#suite_error' do
+    subject(:suite_error) { test_suite_summary.suite_error }
+
+    context 'when there are no build report results with suite errors' do
+      it { is_expected.to be_nil }
+    end
+
+    context 'when there are build report results with suite errors' do
+      let(:build_report_result_1) do
+        build(
+          :ci_build_report_result,
+          :with_junit_suite_error,
+          test_suite_name: 'karma',
+          test_suite_error: 'karma parsing error'
+        )
+      end
+
+      let(:build_report_result_2) do
+        build(
+          :ci_build_report_result,
+          :with_junit_suite_error,
+          test_suite_name: 'karma',
+          test_suite_error: 'another karma parsing error'
+        )
+      end
+
+      it 'includes the first suite error from the collection of build report results' do
+        expect(suite_error).to eq('karma parsing error')
+      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)
+        expect(subject).to include(:time, :count, :success, :failed, :skipped, :error, :suite_error)
       end
     end
   end

spec/serializers/test_suite_summary_entity_spec.rb

@@ -20,5 +20,9 @@ RSpec.describe TestSuiteSummaryEntity do
     it 'contains the build_ids' do
       expect(as_json).to include(:build_ids)
     end
+
+    it 'contains the suite_error' do
+      expect(as_json).to include(:suite_error)
+    end
   end
 end

spec/services/ci/build_report_result_service_spec.rb

@@ -10,13 +10,17 @@ RSpec.describe Ci::BuildReportResultService do
       let(:build) { create(:ci_build, :success, :test_reports) }
 
       it 'creates a build report result entry', :aggregate_failures do
+        expect { build_report_result }.to change { Ci::BuildReportResult.count }.by(1)
         expect(build_report_result.tests_name).to eq("test")
         expect(build_report_result.tests_success).to eq(2)
         expect(build_report_result.tests_failed).to eq(2)
         expect(build_report_result.tests_errored).to eq(0)
         expect(build_report_result.tests_skipped).to eq(0)
         expect(build_report_result.tests_duration).to eq(0.010284)
-        expect(Ci::BuildReportResult.count).to eq(1)
+      end
+
+      it 'tracks unique test cases parsed' do
+        build_report_result
 
         unique_test_cases_parsed = Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(
           event_names: described_class::EVENT_NAME,
@@ -26,6 +30,32 @@ RSpec.describe Ci::BuildReportResultService do
         expect(unique_test_cases_parsed).to eq(4)
       end
 
+      context 'and build has test report parsing errors' do
+        let(:build) { create(:ci_build, :success, :broken_test_reports) }
+
+        it 'creates a build report result entry with suite error', :aggregate_failures do
+          expect { build_report_result }.to change { Ci::BuildReportResult.count }.by(1)
+          expect(build_report_result.tests_name).to eq("test")
+          expect(build_report_result.tests_success).to eq(0)
+          expect(build_report_result.tests_failed).to eq(0)
+          expect(build_report_result.tests_errored).to eq(0)
+          expect(build_report_result.tests_skipped).to eq(0)
+          expect(build_report_result.tests_duration).to eq(0)
+          expect(build_report_result.suite_error).to be_present
+        end
+
+        it 'does not track unique test cases parsed' do
+          build_report_result
+
+          unique_test_cases_parsed = Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(
+            event_names: described_class::EVENT_NAME,
+            start_date: 2.weeks.ago,
+            end_date: 2.weeks.from_now
+          )
+          expect(unique_test_cases_parsed).to eq(0)
+        end
+      end
+
       context 'when data has already been persisted' do
         it 'raises an error and do not persist the same data twice' do
           expect { 2.times { described_class.new.execute(build) } }.to raise_error(ActiveRecord::RecordNotUnique)