Add spec to ensure we are limiting test cases in the comparer

Cleans up the method a bit and also added a spec to ensure
we are limiting test cases in the comparer.

app/services/ci/compare_test_reports_service.rb

@@ -23,13 +23,8 @@ module Ci
     end
 
     def failed_test_cases(test_reports_comparer)
-      [].tap do |test_cases|
-        test_reports_comparer.suite_comparers.each do |suite_comparer|
-          # We're basing off the limited tests because this is what's presented on the MR widget
-          # and at the same time we only want to query for a limited amount of test failures.
-          limited_tests = suite_comparer.limited_tests
-          test_cases.concat(limited_tests.new_failures + limited_tests.existing_failures)
-        end
+      test_reports_comparer.suite_comparers.flat_map do |suite_comparer|
+        suite_comparer.limited_tests.new_failures + suite_comparer.limited_tests.existing_failures
       end
     end
   end

spec/factories/ci/builds.rb

@@ -338,6 +338,12 @@ FactoryBot.define do
       end
     end
 
+    trait :test_reports_with_three_failures do
+      after(:build) do |build|
+        build.job_artifacts << create(:ci_job_artifact, :junit_with_three_failures, job: build)
+      end
+    end
+
     trait :accessibility_reports do
       after(:build) do |build|
         build.job_artifacts << create(:ci_job_artifact, :accessibility, job: build)

spec/factories/ci/job_artifacts.rb

@@ -149,6 +149,16 @@ FactoryBot.define do
       end
     end
 
+    trait :junit_with_three_failures do
+      file_type { :junit }
+      file_format { :gzip }
+
+      after(:build) do |artifact, evaluator|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/junit/junit_with_three_failures.xml.gz'), 'application/x-gzip')
+      end
+    end
+
     trait :accessibility do
       file_type { :accessibility }
       file_format { :raw }

spec/factories/ci/pipelines.rb

@@ -121,6 +121,14 @@ FactoryBot.define do
         end
       end
 
+      trait :with_test_reports_with_three_failures do
+        status { :success }
+
+        after(:build) do |pipeline, _evaluator|
+          pipeline.builds << build(:ci_build, :test_reports_with_three_failures, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
       trait :with_accessibility_reports do
         status { :success }
 

spec/services/ci/compare_test_reports_service_spec.rb

@@ -47,28 +47,36 @@ RSpec.describe Ci::CompareTestReportsService do
 
     context 'test failure history' do
       let!(:base_pipeline) { nil }
-      let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
+      let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports_with_three_failures, project: project) }
+
+      let(:new_failures) do
+        comparison.dig(:data, 'suites', 0, 'new_failures')
+      end
 
       let(:recent_failures_per_test_case) do
-        comparison.dig(:data, 'suites', 0, 'new_failures').map { |f| f['recent_failures'] }
+        new_failures.map { |f| f['recent_failures'] }
       end
 
       # Create test case failure records based on the head pipeline build
       before do
+        stub_const("Gitlab::Ci::Reports::TestSuiteComparer::DEFAULT_MAX_TESTS", 2)
+        stub_const("Gitlab::Ci::Reports::TestSuiteComparer::DEFAULT_MIN_TESTS", 1)
+
         build = head_pipeline.builds.last
         build.update_column(:finished_at, 1.day.ago) # Just to be sure we are included in the report window
 
-        # The JUnit fixture for the given build has 2 failures.
+        # The JUnit fixture for the given build has 3 failures.
         # This service will create 1 test case failure record for each.
         Ci::TestCasesService.new.execute(build)
       end
 
-      it 'loads on the report', :aggregate_failures do
+      it 'loads recent failures on limited test cases to avoid building up a huge DB query', :aggregate_failures do
         expect(comparison[:data]).to match_schema('entities/test_reports_comparer')
         expect(recent_failures_per_test_case).to eq([
           { 'count' => 1, 'base_branch' => 'master' },
           { 'count' => 1, 'base_branch' => 'master' }
         ])
+        expect(new_failures.count).to eq(2)
       end
     end
   end