Limit amount of JUnit tests returned

Currently, we do not cap amount of tests returned to frontend,
thus in some extreme cases we can see a MBs of data stored in Redis.

This adds an upper limit of 100 tests per-suite.

We will continue showing the total counters correctly,
but we will limit amount of tests that will be presented.

app/serializers/test_suite_comparer_entity.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 class TestSuiteComparerEntity < Grape::Entity
+  DEFAULT_MAX_TESTS = 100
+  DEFAULT_MIN_TESTS = 10
+
   expose :name
   expose :total_status, as: :status
 
@@ -10,7 +13,27 @@ class TestSuiteComparerEntity < Grape::Entity
     expose :failed_count, as: :failed
   end
 
-  expose :new_failures, using: TestCaseEntity
-  expose :resolved_failures, using: TestCaseEntity
-  expose :existing_failures, using: TestCaseEntity
+  # rubocop: disable CodeReuse/ActiveRecord
+  expose :new_failures, using: TestCaseEntity do |suite|
+    suite.new_failures.take(max_tests)
+  end
+
+  expose :existing_failures, using: TestCaseEntity do |suite|
+    suite.existing_failures.take(
+      max_tests(suite.new_failures))
+  end
+
+  expose :resolved_failures, using: TestCaseEntity do |suite|
+    suite.resolved_failures.take(
+      max_tests(suite.new_failures, suite.existing_failures))
+  end
+
+  private
+
+  def max_tests(*used)
+    return Integer::MAX unless Feature.enabled?(:ci_limit_test_reports_size, default_enabled: true)
+
+    [DEFAULT_MAX_TESTS - used.map(&:count).sum, DEFAULT_MIN_TESTS].max
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
 end

app/services/ci/compare_reports_base_service.rb

@@ -8,7 +8,7 @@ module Ci
         status: :parsed,
         key: key(base_pipeline, head_pipeline),
         data: serializer_class
-          .new(project: project)
+          .new(**serializer_params)
           .represent(comparer).as_json
       }
     rescue Gitlab::Ci::Parsers::ParserError => e
@@ -40,6 +40,10 @@ module Ci
       raise NotImplementedError
     end
 
+    def serializer_params
+      { project: project }
+    end
+
     def get_report(pipeline)
       raise NotImplementedError
     end

changelogs/unreleased/limit-amount-of-tests-returned.yml

@@ -0,0 +1,5 @@
+---
+title: Limit amount of JUnit tests returned
+merge_request: 30274
+author:
+type: performance

spec/features/merge_request/user_sees_merge_widget_spec.rb

@@ -519,6 +519,8 @@ describe 'Merge request > User sees merge widget', :js do
       end
 
       before do
+        allow_any_instance_of(TestSuiteComparerEntity)
+          .to receive(:max_tests).and_return(2)
         allow_any_instance_of(MergeRequest)
           .to receive(:has_test_reports?).and_return(true)
         allow_any_instance_of(MergeRequest)
@@ -551,7 +553,7 @@ describe 'Merge request > User sees merge widget', :js do
               expect(page).to have_content('rspec found no changed test results out of 1 total test')
               expect(page).to have_content('junit found 1 failed test result out of 1 total test')
               expect(page).to have_content('New')
-              expect(page).to have_content('subtractTest')
+              expect(page).to have_content('addTest')
             end
           end
         end
@@ -562,7 +564,7 @@ describe 'Merge request > User sees merge widget', :js do
               click_button 'Expand'
 
               within(".js-report-section-container") do
-                click_button 'subtractTest'
+                click_button 'addTest'
 
                 expect(page).to have_content('6.66')
                 expect(page).to have_content(sample_java_failed_message.gsub!(/\s+/, ' ').strip)
@@ -596,7 +598,7 @@ describe 'Merge request > User sees merge widget', :js do
               expect(page).to have_content('rspec found 1 failed test result out of 1 total test')
               expect(page).to have_content('junit found no changed test results out of 1 total test')
               expect(page).not_to have_content('New')
-              expect(page).to have_content('Test#sum when a is 2 and b is 2 returns summary')
+              expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary')
             end
           end
         end
@@ -607,7 +609,7 @@ describe 'Merge request > User sees merge widget', :js do
               click_button 'Expand'
 
               within(".js-report-section-container") do
-                click_button 'Test#sum when a is 2 and b is 2 returns summary'
+                click_button 'Test#sum when a is 1 and b is 3 returns summary'
 
                 expect(page).to have_content('2.22')
                 expect(page).to have_content(sample_rspec_failed_message.gsub!(/\s+/, ' ').strip)
@@ -628,13 +630,7 @@ describe 'Merge request > User sees merge widget', :js do
         let(:head_reports) do
           Gitlab::Ci::Reports::TestReports.new.tap do |reports|
             reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
-            reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
-          end
-        end
-
-        let(:create_test_case_java_resolved) do
-          create_test_case_java_failed.tap do |test_case|
-            test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
+            reports.get_suite('junit').add_test_case(create_test_case_java_success)
           end
         end
 
@@ -646,7 +642,7 @@ describe 'Merge request > User sees merge widget', :js do
             within(".js-report-section-container") do
               expect(page).to have_content('rspec found no changed test results out of 1 total test')
               expect(page).to have_content('junit found 1 fixed test result out of 1 total test')
-              expect(page).to have_content('subtractTest')
+              expect(page).to have_content('addTest')
             end
           end
         end
@@ -657,15 +653,53 @@ describe 'Merge request > User sees merge widget', :js do
               click_button 'Expand'
 
               within(".js-report-section-container") do
-                click_button 'subtractTest'
+                click_button 'addTest'
 
-                expect(page).to have_content('6.66')
+                expect(page).to have_content('5.55')
               end
             end
           end
         end
       end
 
+      context 'properly truncates the report' do
+        let(:base_reports) do
+          Gitlab::Ci::Reports::TestReports.new.tap do |reports|
+            10.times do |index|
+              reports.get_suite('rspec').add_test_case(
+                create_test_case_rspec_failed(index))
+              reports.get_suite('junit').add_test_case(
+                create_test_case_java_success(index))
+            end
+          end
+        end
+
+        let(:head_reports) do
+          Gitlab::Ci::Reports::TestReports.new.tap do |reports|
+            10.times do |index|
+              reports.get_suite('rspec').add_test_case(
+                create_test_case_rspec_failed(index))
+              reports.get_suite('junit').add_test_case(
+                create_test_case_java_failed(index))
+            end
+          end
+        end
+
+        it 'shows test reports summary which includes the resolved failure' do
+          within(".js-reports-container") do
+            click_button 'Expand'
+
+            expect(page).to have_content('Test summary contained 20 failed test results out of 20 total tests')
+            within(".js-report-section-container") do
+              expect(page).to have_content('rspec found 10 failed test results out of 10 total tests')
+              expect(page).to have_content('junit found 10 failed test results out of 10 total tests')
+
+              expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary', count: 2)
+            end
+          end
+        end
+      end
+
       def comparer
         Gitlab::Ci::Reports::TestReportsComparer.new(base_reports, head_reports)
       end

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

@@ -74,17 +74,11 @@ describe Gitlab::Ci::Reports::TestReportsComparer do
     subject { comparer.resolved_count }
 
     context 'when there is a resolved test case in head suites' do
-      let(:create_test_case_java_resolved) do
-        create_test_case_java_failed.tap do |test_case|
-          test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
-        end
-      end
-
       before do
         base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
         base_reports.get_suite('junit').add_test_case(create_test_case_java_failed)
         head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
-        head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
+        head_reports.get_suite('junit').add_test_case(create_test_case_java_success)
       end
 
       it 'returns the correct count' do

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

@@ -10,12 +10,6 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
   let(:test_case_success) { create_test_case_rspec_success }
   let(:test_case_failed) { create_test_case_rspec_failed }
 
-  let(:test_case_resolved) do
-    create_test_case_rspec_failed.tap do |test_case|
-      test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
-    end
-  end
-
   describe '#new_failures' do
     subject { comparer.new_failures }
 
@@ -44,7 +38,7 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
     context 'when head sutie has a success test case which failed in base' do
       before do
         base_suite.add_test_case(test_case_failed)
-        head_suite.add_test_case(test_case_resolved)
+        head_suite.add_test_case(test_case_success)
       end
 
       it 'does not return the failed test case' do
@@ -81,7 +75,7 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
     context 'when head sutie has a success test case which failed in base' do
       before do
         base_suite.add_test_case(test_case_failed)
-        head_suite.add_test_case(test_case_resolved)
+        head_suite.add_test_case(test_case_success)
       end
 
       it 'does not return the failed test case' do
@@ -126,11 +120,11 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
     context 'when head sutie has a success test case which failed in base' do
       before do
         base_suite.add_test_case(test_case_failed)
-        head_suite.add_test_case(test_case_resolved)
+        head_suite.add_test_case(test_case_success)
       end
 
       it 'does not return the resolved test case' do
-        is_expected.to eq([test_case_resolved])
+        is_expected.to eq([test_case_success])
       end
 
       it 'returns the correct resolved count' do
@@ -156,13 +150,8 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
 
     context 'when there are a new failure and an existing failure' do
       let(:test_case_1_success) { create_test_case_rspec_success }
-      let(:test_case_2_failed) { create_test_case_rspec_failed }
-
-      let(:test_case_1_failed) do
-        create_test_case_rspec_success.tap do |test_case|
-          test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_FAILED)
-        end
-      end
+      let(:test_case_1_failed) { create_test_case_rspec_failed }
+      let(:test_case_2_failed) { create_test_case_rspec_failed('case2') }
 
       before do
         base_suite.add_test_case(test_case_1_success)

spec/serializers/test_case_entity_spec.rb

@@ -24,7 +24,7 @@ describe TestCaseEntity do
 
       it 'contains correct test case details' do
         expect(subject[:status]).to eq('failed')
-        expect(subject[:name]).to eq('Test#sum when a is 2 and b is 2 returns summary')
+        expect(subject[:name]).to eq('Test#sum when a is 1 and b is 3 returns summary')
         expect(subject[:classname]).to eq('spec.test_spec')
         expect(subject[:execution_time]).to eq(2.22)
       end

spec/serializers/test_reports_comparer_entity_spec.rb

@@ -53,13 +53,7 @@ describe TestReportsComparerEntity do
           base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
           base_reports.get_suite('junit').add_test_case(create_test_case_java_failed)
           head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
-          head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
-        end
-
-        let(:create_test_case_java_resolved) do
-          create_test_case_java_failed.tap do |test_case|
-            test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
-          end
+          head_reports.get_suite('junit').add_test_case(create_test_case_java_success)
         end
 
         it 'contains correct compared test reports details' do

spec/serializers/test_reports_comparer_serializer_spec.rb

@@ -44,13 +44,7 @@ describe TestReportsComparerSerializer do
           base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
           base_reports.get_suite('junit').add_test_case(create_test_case_java_failed)
           head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
-          head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
-        end
-
-        let(:create_test_case_java_resolved) do
-          create_test_case_java_failed.tap do |test_case|
-            test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
-          end
+          head_reports.get_suite('junit').add_test_case(create_test_case_java_success)
         end
 
         it 'matches the schema' do

spec/serializers/test_suite_comparer_entity_spec.rb

@@ -11,16 +11,10 @@ describe TestSuiteComparerEntity do
   let(:test_case_success) { create_test_case_rspec_success }
   let(:test_case_failed) { create_test_case_rspec_failed }
 
-  let(:test_case_resolved) do
-    create_test_case_rspec_failed.tap do |test_case|
-      test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
-    end
-  end
-
   describe '#as_json' do
     subject { entity.as_json }
 
-    context 'when head sutie has a newly failed test case which does not exist in base' do
+    context 'when head suite has a newly failed test case which does not exist in base' do
       before do
         base_suite.add_test_case(test_case_success)
         head_suite.add_test_case(test_case_failed)
@@ -41,7 +35,7 @@ describe TestSuiteComparerEntity do
       end
     end
 
-    context 'when head sutie still has a failed test case which failed in base' do
+    context 'when head suite still has a failed test case which failed in base' do
       before do
         base_suite.add_test_case(test_case_failed)
         head_suite.add_test_case(test_case_failed)
@@ -62,10 +56,10 @@ describe TestSuiteComparerEntity do
       end
     end
 
-    context 'when head sutie has a success test case which failed in base' do
+    context 'when head suite has a success test case which failed in base' do
       before do
         base_suite.add_test_case(test_case_failed)
-        head_suite.add_test_case(test_case_resolved)
+        head_suite.add_test_case(test_case_success)
       end
 
       it 'contains correct compared test suite details' do
@@ -74,13 +68,83 @@ describe TestSuiteComparerEntity do
         expect(subject[:summary]).to include(total: 1, resolved: 1, failed: 0)
         expect(subject[:new_failures]).to be_empty
         subject[:resolved_failures].first.tap do |resolved_failure|
-          expect(resolved_failure[:status]).to eq(test_case_resolved.status)
-          expect(resolved_failure[:name]).to eq(test_case_resolved.name)
-          expect(resolved_failure[:execution_time]).to eq(test_case_resolved.execution_time)
-          expect(resolved_failure[:system_output]).to eq(test_case_resolved.system_output)
+          expect(resolved_failure[:status]).to eq(test_case_success.status)
+          expect(resolved_failure[:name]).to eq(test_case_success.name)
+          expect(resolved_failure[:execution_time]).to eq(test_case_success.execution_time)
+          expect(resolved_failure[:system_output]).to eq(test_case_success.system_output)
         end
         expect(subject[:existing_failures]).to be_empty
       end
     end
+
+    context 'limits amount of tests returned' do
+      before do
+        stub_const('TestSuiteComparerEntity::DEFAULT_MAX_TESTS', 2)
+        stub_const('TestSuiteComparerEntity::DEFAULT_MIN_TESTS', 1)
+      end
+
+      context 'prefers new over existing and resolved' do
+        before do
+          3.times { add_new_failure }
+          3.times { add_existing_failure }
+          3.times { add_resolved_failure }
+        end
+
+        it 'returns 2 new failures, and 1 of resolved and existing' do
+          expect(subject[:summary]).to include(total: 9, resolved: 3, failed: 6)
+          expect(subject[:new_failures].count).to eq(2)
+          expect(subject[:existing_failures].count).to eq(1)
+          expect(subject[:resolved_failures].count).to eq(1)
+        end
+      end
+
+      context 'prefers existing over resolved' do
+        before do
+          3.times { add_existing_failure }
+          3.times { add_resolved_failure }
+        end
+
+        it 'returns 2 existing failures, and 1 resolved' do
+          expect(subject[:summary]).to include(total: 6, resolved: 3, failed: 3)
+          expect(subject[:new_failures].count).to eq(0)
+          expect(subject[:existing_failures].count).to eq(2)
+          expect(subject[:resolved_failures].count).to eq(1)
+        end
+      end
+
+      context 'limits amount of resolved' do
+        before do
+          3.times { add_resolved_failure }
+        end
+
+        it 'returns 2 resolved failures' do
+          expect(subject[:summary]).to include(total: 3, resolved: 3, failed: 0)
+          expect(subject[:new_failures].count).to eq(0)
+          expect(subject[:existing_failures].count).to eq(0)
+          expect(subject[:resolved_failures].count).to eq(2)
+        end
+      end
+
+      private
+
+      def add_new_failure
+        failed_case = create_test_case_rspec_failed(SecureRandom.hex)
+        head_suite.add_test_case(failed_case)
+      end
+
+      def add_existing_failure
+        failed_case = create_test_case_rspec_failed(SecureRandom.hex)
+        base_suite.add_test_case(failed_case)
+        head_suite.add_test_case(failed_case)
+      end
+
+      def add_resolved_failure
+        case_name = SecureRandom.hex
+        failed_case = create_test_case_rspec_failed(case_name)
+        success_case = create_test_case_rspec_success(case_name)
+        base_suite.add_test_case(failed_case)
+        head_suite.add_test_case(success_case)
+      end
+    end
   end
 end

spec/support/test_reports/test_reports_helper.rb

@@ -1,36 +1,36 @@
 module TestReportsHelper
-  def create_test_case_rspec_success
+  def create_test_case_rspec_success(name = 'test_spec')
     Gitlab::Ci::Reports::TestCase.new(
       name: 'Test#sum when a is 1 and b is 3 returns summary',
-      classname: 'spec.test_spec',
+      classname: "spec.#{name}",
       file: './spec/test_spec.rb',
       execution_time: 1.11,
       status: Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
   end
 
-  def create_test_case_rspec_failed
+  def create_test_case_rspec_failed(name = 'test_spec')
     Gitlab::Ci::Reports::TestCase.new(
-      name: 'Test#sum when a is 2 and b is 2 returns summary',
-      classname: 'spec.test_spec',
+      name: 'Test#sum when a is 1 and b is 3 returns summary',
+      classname: "spec.#{name}",
       file: './spec/test_spec.rb',
       execution_time: 2.22,
       system_output: sample_rspec_failed_message,
       status: Gitlab::Ci::Reports::TestCase::STATUS_FAILED)
   end
 
-  def create_test_case_rspec_skipped
+  def create_test_case_rspec_skipped(name = 'test_spec')
     Gitlab::Ci::Reports::TestCase.new(
       name: 'Test#sum when a is 3 and b is 3 returns summary',
-      classname: 'spec.test_spec',
+      classname: "spec.#{name}",
       file: './spec/test_spec.rb',
       execution_time: 3.33,
       status: Gitlab::Ci::Reports::TestCase::STATUS_SKIPPED)
   end
 
-  def create_test_case_rspec_error
+  def create_test_case_rspec_error(name = 'test_spec')
     Gitlab::Ci::Reports::TestCase.new(
       name: 'Test#sum when a is 4 and b is 4 returns summary',
-      classname: 'spec.test_spec',
+      classname: "spec.#{name}",
       file: './spec/test_spec.rb',
       execution_time: 4.44,
       status: Gitlab::Ci::Reports::TestCase::STATUS_ERROR)
@@ -48,34 +48,34 @@ module TestReportsHelper
     EOF
   end
 
-  def create_test_case_java_success
+  def create_test_case_java_success(name = 'addTest')
     Gitlab::Ci::Reports::TestCase.new(
-      name: 'addTest',
+      name: name,
       classname: 'CalculatorTest',
       execution_time: 5.55,
       status: Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
   end
 
-  def create_test_case_java_failed
+  def create_test_case_java_failed(name = 'addTest')
     Gitlab::Ci::Reports::TestCase.new(
-      name: 'subtractTest',
+      name: name,
       classname: 'CalculatorTest',
       execution_time: 6.66,
       system_output: sample_java_failed_message,
       status: Gitlab::Ci::Reports::TestCase::STATUS_FAILED)
   end
 
-  def create_test_case_java_skipped
+  def create_test_case_java_skipped(name = 'addTest')
     Gitlab::Ci::Reports::TestCase.new(
-      name: 'multiplyTest',
+      name: name,
       classname: 'CalculatorTest',
       execution_time: 7.77,
       status: Gitlab::Ci::Reports::TestCase::STATUS_SKIPPED)
   end
 
-  def create_test_case_java_error
+  def create_test_case_java_error(name = 'addTest')
     Gitlab::Ci::Reports::TestCase.new(
-      name: 'divideTest',
+      name: name,
       classname: 'CalculatorTest',
       execution_time: 8.88,
       status: Gitlab::Ci::Reports::TestCase::STATUS_ERROR)