Merge branch 'eb-test-failure-history-mvc-api' into 'master'

Include recent failures data in test reports response

See merge request gitlab-org/gitlab!45187

app/models/ci/test_case_failure.rb

@@ -4,9 +4,26 @@ module Ci
   class TestCaseFailure < ApplicationRecord
     extend Gitlab::Ci::Model
 
+    REPORT_WINDOW = 14.days
+
     validates :test_case, :build, :failed_at, presence: true
 
     belongs_to :test_case, class_name: "Ci::TestCase", foreign_key: :test_case_id
     belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id
+
+    def self.recent_failures_count(project:, test_case_keys:, date_range: REPORT_WINDOW.ago..Time.current)
+      joins(:test_case)
+        .where(
+          ci_test_cases: {
+            project_id: project.id,
+            key_hash: test_case_keys
+          },
+          ci_test_case_failures: {
+            failed_at: date_range
+          }
+        )
+        .group(:key_hash)
+        .count('ci_test_case_failures.id')
+    end
   end
 end

app/serializers/test_case_entity.rb

@@ -10,6 +10,7 @@ class TestCaseEntity < Grape::Entity
   expose :execution_time
   expose :system_output
   expose :stack_trace
+  expose :recent_failures
   expose :attachment_url, if: -> (*) { can_read_screenshots? } do |test_case|
     expose_url(test_case.attachment_url)
   end

app/serializers/test_suite_comparer_entity.rb

 # frozen_string_literal: true
 
 class TestSuiteComparerEntity < Grape::Entity
-  DEFAULT_MAX_TESTS = 100
-  DEFAULT_MIN_TESTS = 10
-
   expose :name
   expose :total_status, as: :status
 
@@ -14,39 +11,27 @@ class TestSuiteComparerEntity < Grape::Entity
     expose :error_count, as: :errored
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   expose :new_failures, using: TestCaseEntity do |suite|
-    suite.new_failures.take(max_tests)
+    suite.limited_tests.new_failures
   end
 
   expose :existing_failures, using: TestCaseEntity do |suite|
-    suite.existing_failures.take(
-      max_tests(suite.new_failures))
+    suite.limited_tests.existing_failures
   end
 
   expose :resolved_failures, using: TestCaseEntity do |suite|
-    suite.resolved_failures.take(
-      max_tests(suite.new_failures, suite.existing_failures))
+    suite.limited_tests.resolved_failures
   end
 
   expose :new_errors, using: TestCaseEntity do |suite|
-    suite.new_errors.take(max_tests)
+    suite.limited_tests.new_errors
   end
 
   expose :existing_errors, using: TestCaseEntity do |suite|
-    suite.existing_errors.take(
-      max_tests(suite.new_errors))
+    suite.limited_tests.existing_errors
   end
 
   expose :resolved_errors, using: TestCaseEntity do |suite|
-    suite.resolved_errors.take(
-      max_tests(suite.new_errors, suite.existing_errors))
-  end
-
-  private
-
-  def max_tests(*used)
-    [DEFAULT_MAX_TESTS - used.map(&:count).sum, DEFAULT_MIN_TESTS].max
+    suite.limited_tests.resolved_errors
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 end

app/services/ci/compare_reports_base_service.rb

@@ -8,7 +8,9 @@ module Ci
   # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224
   class CompareReportsBaseService < ::BaseService
     def execute(base_pipeline, head_pipeline)
-      comparer = build_comparer(base_pipeline, head_pipeline)
+      base_report = get_report(base_pipeline)
+      head_report = get_report(head_pipeline)
+      comparer = build_comparer(base_report, head_report)
 
       {
         status: :parsed,
@@ -31,8 +33,8 @@ module Ci
 
     protected
 
-    def build_comparer(base_pipeline, head_pipeline)
-      comparer_class.new(get_report(base_pipeline), get_report(head_pipeline))
+    def build_comparer(base_report, head_report)
+      comparer_class.new(base_report, head_report)
     end
 
     private

app/services/ci/compare_test_reports_service.rb

@@ -13,5 +13,19 @@ module Ci
     def get_report(pipeline)
       pipeline&.test_reports
     end
+
+    def build_comparer(base_report, head_report)
+      # We need to load the test failure history on the test comparer because we display
+      # this on the MR widget
+      super.tap do |test_reports_comparer|
+        ::Gitlab::Ci::Reports::TestFailureHistory.new(failed_test_cases(test_reports_comparer), project).load!
+      end
+    end
+
+    def failed_test_cases(test_reports_comparer)
+      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
 end

lib/gitlab/ci/reports/test_case.rb

@@ -10,7 +10,7 @@ module Gitlab
         STATUS_ERROR = 'error'
         STATUS_TYPES = [STATUS_ERROR, STATUS_FAILED, STATUS_SUCCESS, STATUS_SKIPPED].freeze
 
-        attr_reader :suite_name, :name, :classname, :execution_time, :status, :file, :system_output, :stack_trace, :key, :attachment, :job
+        attr_reader :suite_name, :name, :classname, :execution_time, :status, :file, :system_output, :stack_trace, :key, :attachment, :job, :recent_failures
 
         def initialize(params)
           @suite_name = params.fetch(:suite_name)
@@ -24,9 +24,15 @@ module Gitlab
           @attachment = params.fetch(:attachment, nil)
           @job = params.fetch(:job, nil)
 
+          @recent_failures = nil
+
           @key = hash_key("#{suite_name}_#{classname}_#{name}")
         end
 
+        def set_recent_failures(count, base_branch)
+          @recent_failures = { count: count, base_branch: base_branch }
+        end
+
         def has_attachment?
           attachment.present?
         end

lib/gitlab/ci/reports/test_failure_history.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Reports
+      class TestFailureHistory
+        include Gitlab::Utils::StrongMemoize
+
+        def initialize(failed_test_cases, project)
+          @failed_test_cases = build_map(failed_test_cases)
+          @project = project
+        end
+
+        def load!
+          return unless Feature.enabled?(:test_failure_history, project)
+
+          recent_failures_count.each do |key_hash, count|
+            failed_test_cases[key_hash].set_recent_failures(count, project.default_branch_or_master)
+          end
+        end
+
+        private
+
+        attr_reader :report, :project, :failed_test_cases
+
+        def recent_failures_count
+          ::Ci::TestCaseFailure.recent_failures_count(
+            project: project,
+            test_case_keys: failed_test_cases.keys
+          )
+        end
+
+        def build_map(test_cases)
+          {}.tap do |hash|
+            test_cases.each do |test_case|
+              hash[test_case.key] = test_case
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/reports/test_suite_comparer.rb

@@ -6,6 +6,9 @@ module Gitlab
       class TestSuiteComparer
         include Gitlab::Utils::StrongMemoize
 
+        DEFAULT_MAX_TESTS = 100
+        DEFAULT_MIN_TESTS = 10
+
         attr_reader :name, :base_suite, :head_suite
 
         def initialize(name, base_suite, head_suite)
@@ -81,6 +84,29 @@ module Gitlab
         def error_count
           new_errors.count + existing_errors.count
         end
+
+        # This is used to limit the presented test cases but does not affect
+        # total count of tests in the summary
+        def limited_tests
+          strong_memoize(:limited_tests) do
+            # rubocop: disable CodeReuse/ActiveRecord
+            OpenStruct.new(
+              new_failures: new_failures.take(max_tests),
+              existing_failures: existing_failures.take(max_tests(new_failures)),
+              resolved_failures: resolved_failures.take(max_tests(new_failures, existing_failures)),
+              new_errors: new_errors.take(max_tests),
+              existing_errors: existing_errors.take(max_tests(new_errors)),
+              resolved_errors: resolved_errors.take(max_tests(new_errors, existing_errors))
+            )
+            # rubocop: enable CodeReuse/ActiveRecord
+          end
+        end
+
+        private
+
+        def max_tests(*used)
+          [DEFAULT_MAX_TESTS - used.map(&:count).sum, DEFAULT_MIN_TESTS].max
+        end
       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/features/merge_request/user_sees_merge_widget_spec.rb

@@ -558,8 +558,9 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
       end
 
       before do
-        allow_any_instance_of(TestSuiteComparerEntity)
-          .to receive(:max_tests).and_return(2)
+        stub_const("Gitlab::Ci::Reports::TestSuiteComparer::DEFAULT_MAX_TESTS", 2)
+        stub_const("Gitlab::Ci::Reports::TestSuiteComparer::DEFAULT_MIN_TESTS", 1)
+
         allow_any_instance_of(MergeRequest)
           .to receive(:has_test_reports?).and_return(true)
         allow_any_instance_of(MergeRequest)

spec/fixtures/api/schemas/entities/test_case.json

@@ -12,7 +12,13 @@
     "execution_time": { "type": "float" },
     "system_output": { "type": ["string", "null"] },
     "stack_trace":  { "type": ["string", "null"] },
-    "attachment_url":  { "type": ["string", "null"] }
+    "attachment_url":  { "type": ["string", "null"] },
+    "recent_failures": {
+      "oneOf": [
+        { "type": "null" },
+        { "$ref": "test_case/recent_failures.json" }
+      ]
+    }
   },
   "additionalProperties": false
 }

spec/fixtures/api/schemas/entities/test_case/recent_failures.json

+{
+  "type": "object",
+  "required": [
+    "count",
+    "base_branch"
+  ],
+  "properties": {
+    "count": { "type": "integer" },
+    "base_branch": { "type": "string" }
+  },
+  "additionalProperties": false
+}

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

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Ci::Reports::TestCase do
+RSpec.describe Gitlab::Ci::Reports::TestCase, :aggregate_failures do
   describe '#initialize' do
     let(:test_case) { described_class.new(params) }
 
@@ -82,4 +82,17 @@ RSpec.describe Gitlab::Ci::Reports::TestCase do
       end
     end
   end
+
+  describe '#set_recent_failures' do
+    it 'sets the recent_failures information' do
+      test_case = build(:report_test_case)
+
+      test_case.set_recent_failures(1, 'master')
+
+      expect(test_case.recent_failures).to eq(
+        count: 1,
+        base_branch: 'master'
+      )
+    end
+  end
 end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Reports::TestFailureHistory, :aggregate_failures do
+  include TestReportsHelper
+
+  describe '#load!' do
+    let_it_be(:project) { create(:project) }
+    let(:failed_rspec) { create_test_case_rspec_failed }
+    let(:failed_java) { create_test_case_java_failed }
+
+    subject(:load_history) { described_class.new([failed_rspec, failed_java], project).load! }
+
+    before do
+      allow(Ci::TestCaseFailure)
+        .to receive(:recent_failures_count)
+        .with(project: project, test_case_keys: [failed_rspec.key, failed_java.key])
+        .and_return(
+          failed_rspec.key => 2,
+          failed_java.key => 1
+        )
+    end
+
+    it 'sets the recent failures for each matching failed test case in all test suites' do
+      load_history
+
+      expect(failed_rspec.recent_failures).to eq(count: 2, base_branch: 'master')
+      expect(failed_java.recent_failures).to eq(count: 1, base_branch: 'master')
+    end
+
+    context 'when feature flag is disabled' do
+      before do
+        stub_feature_flags(test_failure_history: false)
+      end
+
+      it 'does not set recent failures' do
+        load_history
+
+        expect(failed_rspec.recent_failures).to be_nil
+        expect(failed_java.recent_failures).to be_nil
+      end
+    end
+  end
+end

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

@@ -2,11 +2,11 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer do
+RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer, :aggregate_failures do
   include TestReportsHelper
 
   let(:comparer) { described_class.new(name, base_suite, head_suite) }
-  let(:name) { 'rpsec' }
+  let(:name) { 'rspec' }
   let(:base_suite) { Gitlab::Ci::Reports::TestSuite.new(name) }
   let(:head_suite) { Gitlab::Ci::Reports::TestSuite.new(name) }
   let(:test_case_success) { create_test_case_java_success }
@@ -16,7 +16,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer do
   describe '#new_failures' do
     subject { comparer.new_failures }
 
-    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)
@@ -27,7 +27,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer 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)
@@ -38,7 +38,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer 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_success)
@@ -53,7 +53,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer do
   describe '#existing_failures' do
     subject { comparer.existing_failures }
 
-    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)
@@ -64,7 +64,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer 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)
@@ -75,7 +75,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer 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_success)
@@ -90,7 +90,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer do
   describe '#resolved_failures' do
     subject { comparer.resolved_failures }
 
-    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)
@@ -105,7 +105,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer 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)
@@ -120,7 +120,7 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer 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_success)
@@ -347,4 +347,128 @@ RSpec.describe Gitlab::Ci::Reports::TestSuiteComparer do
       end
     end
   end
+
+  describe '#limited_tests' do
+    subject(:limited_tests) { comparer.limited_tests }
+
+    context 'limits amount of tests returned' do
+      before do
+        stub_const("#{described_class}::DEFAULT_MAX_TESTS", 2)
+        stub_const("#{described_class}::DEFAULT_MIN_TESTS", 1)
+      end
+
+      context 'prefers new over existing and resolved' do
+        before do
+          3.times { add_new_failure }
+          3.times { add_new_error }
+          3.times { add_existing_failure }
+          3.times { add_existing_error }
+          3.times { add_resolved_failure }
+          3.times { add_resolved_error }
+        end
+
+        it 'returns 2 of each new category, and 1 of each resolved and existing' do
+          expect(limited_tests.new_failures.count).to eq(2)
+          expect(limited_tests.new_errors.count).to eq(2)
+          expect(limited_tests.existing_failures.count).to eq(1)
+          expect(limited_tests.existing_errors.count).to eq(1)
+          expect(limited_tests.resolved_failures.count).to eq(1)
+          expect(limited_tests.resolved_errors.count).to eq(1)
+        end
+
+        it 'does not affect the overall count' do
+          expect(summary).to include(total: 18, resolved: 6, failed: 6, errored: 6)
+        end
+      end
+
+      context 'prefers existing over resolved' do
+        before do
+          3.times { add_existing_failure }
+          3.times { add_existing_error }
+          3.times { add_resolved_failure }
+          3.times { add_resolved_error }
+        end
+
+        it 'returns 2 of each existing category, and 1 of each resolved' do
+          expect(limited_tests.new_failures.count).to eq(0)
+          expect(limited_tests.new_errors.count).to eq(0)
+          expect(limited_tests.existing_failures.count).to eq(2)
+          expect(limited_tests.existing_errors.count).to eq(2)
+          expect(limited_tests.resolved_failures.count).to eq(1)
+          expect(limited_tests.resolved_errors.count).to eq(1)
+        end
+
+        it 'does not affect the overall count' do
+          expect(summary).to include(total: 12, resolved: 6, failed: 3, errored: 3)
+        end
+      end
+
+      context 'limits amount of resolved' do
+        before do
+          3.times { add_resolved_failure }
+          3.times { add_resolved_error }
+        end
+
+        it 'returns 2 of each resolved category' do
+          expect(limited_tests.new_failures.count).to eq(0)
+          expect(limited_tests.new_errors.count).to eq(0)
+          expect(limited_tests.existing_failures.count).to eq(0)
+          expect(limited_tests.existing_errors.count).to eq(0)
+          expect(limited_tests.resolved_failures.count).to eq(2)
+          expect(limited_tests.resolved_errors.count).to eq(2)
+        end
+
+        it 'does not affect the overall count' do
+          expect(summary).to include(total: 6, resolved: 6, failed: 0, errored: 0)
+        end
+      end
+    end
+
+    def summary
+      {
+        total: comparer.total_count,
+        resolved: comparer.resolved_count,
+        failed: comparer.failed_count,
+        errored: comparer.error_count
+      }
+    end
+
+    def add_new_failure
+      failed_case = create_test_case_rspec_failed(SecureRandom.hex)
+      head_suite.add_test_case(failed_case)
+    end
+
+    def add_new_error
+      error_case = create_test_case_rspec_error(SecureRandom.hex)
+      head_suite.add_test_case(error_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_existing_error
+      error_case = create_test_case_rspec_error(SecureRandom.hex)
+      base_suite.add_test_case(error_case)
+      head_suite.add_test_case(error_case)
+    end
+
+    def add_resolved_failure
+      case_name = SecureRandom.hex
+      failed_case = create_test_case_java_failed(case_name)
+      success_case = create_test_case_java_success(case_name)
+      base_suite.add_test_case(failed_case)
+      head_suite.add_test_case(success_case)
+    end
+
+    def add_resolved_error
+      case_name = SecureRandom.hex
+      error_case = create_test_case_java_error(case_name)
+      success_case = create_test_case_java_success(case_name)
+      base_suite.add_test_case(error_case)
+      head_suite.add_test_case(success_case)
+    end
+  end
 end

spec/models/ci/test_case_failure_spec.rb

@@ -15,4 +15,59 @@ RSpec.describe Ci::TestCaseFailure do
     it { is_expected.to validate_presence_of(:build) }
     it { is_expected.to validate_presence_of(:failed_at) }
   end
+
+  describe '.recent_failures_count' do
+    let_it_be(:project) { create(:project) }
+
+    subject(:recent_failures) do
+      described_class.recent_failures_count(
+        project: project,
+        test_case_keys: test_case_keys
+      )
+    end
+
+    context 'when test case failures are within the date range and are for the test case keys' do
+      let(:tc_1) { create(:ci_test_case, project: project) }
+      let(:tc_2) { create(:ci_test_case, project: project) }
+      let(:test_case_keys) { [tc_1.key_hash, tc_2.key_hash] }
+
+      before do
+        create_list(:ci_test_case_failure, 3, test_case: tc_1, failed_at: 1.day.ago)
+        create_list(:ci_test_case_failure, 2, test_case: tc_2, failed_at: 3.days.ago)
+      end
+
+      it 'returns the number of failures for each test case key hash for the past 14 days by default' do
+        expect(recent_failures).to eq(
+          tc_1.key_hash => 3,
+          tc_2.key_hash => 2
+        )
+      end
+    end
+
+    context 'when test case failures are within the date range but are not for the test case keys' do
+      let(:tc) { create(:ci_test_case, project: project) }
+      let(:test_case_keys) { ['some-other-key-hash'] }
+
+      before do
+        create(:ci_test_case_failure, test_case: tc, failed_at: 1.day.ago)
+      end
+
+      it 'excludes them from the count' do
+        expect(recent_failures[tc.key_hash]).to be_nil
+      end
+    end
+
+    context 'when test case failures are not within the date range but are for the test case keys' do
+      let(:tc) { create(:ci_test_case, project: project) }
+      let(:test_case_keys) { [tc.key_hash] }
+
+      before do
+        create(:ci_test_case_failure, test_case: tc, failed_at: 15.days.ago)
+      end
+
+      it 'excludes them from the count' do
+        expect(recent_failures[tc.key_hash]).to be_nil
+      end
+    end
+  end
 end

spec/serializers/test_case_entity_spec.rb

@@ -27,12 +27,17 @@ RSpec.describe TestCaseEntity do
     context 'when test case is failed' do
       let(:test_case) { create_test_case_rspec_failed }
 
+      before do
+        test_case.set_recent_failures(3, 'master')
+      end
+
       it 'contains correct test case details' do
         expect(subject[:status]).to eq('failed')
         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[:file]).to eq('./spec/test_spec.rb')
         expect(subject[:execution_time]).to eq(2.22)
+        expect(subject[:recent_failures]).to eq({ count: 3, base_branch: 'master' })
       end
     end
 

spec/serializers/test_suite_comparer_entity_spec.rb

@@ -100,109 +100,5 @@ RSpec.describe TestSuiteComparerEntity do
         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_new_error }
-          3.times { add_existing_failure }
-          3.times { add_existing_error }
-          3.times { add_resolved_failure }
-          3.times { add_resolved_error }
-        end
-
-        it 'returns 2 of each new category, and 1 of each resolved and existing' do
-          expect(subject[:summary]).to include(total: 18, resolved: 6, failed: 6, errored: 6)
-          expect(subject[:new_failures].count).to eq(2)
-          expect(subject[:new_errors].count).to eq(2)
-          expect(subject[:existing_failures].count).to eq(1)
-          expect(subject[:existing_errors].count).to eq(1)
-          expect(subject[:resolved_failures].count).to eq(1)
-          expect(subject[:resolved_errors].count).to eq(1)
-        end
-      end
-
-      context 'prefers existing over resolved' do
-        before do
-          3.times { add_existing_failure }
-          3.times { add_existing_error }
-          3.times { add_resolved_failure }
-          3.times { add_resolved_error }
-        end
-
-        it 'returns 2 of each existing category, and 1 of each resolved' do
-          expect(subject[:summary]).to include(total: 12, resolved: 6, failed: 3, errored: 3)
-          expect(subject[:new_failures].count).to eq(0)
-          expect(subject[:new_errors].count).to eq(0)
-          expect(subject[:existing_failures].count).to eq(2)
-          expect(subject[:existing_errors].count).to eq(2)
-          expect(subject[:resolved_failures].count).to eq(1)
-          expect(subject[:resolved_errors].count).to eq(1)
-        end
-      end
-
-      context 'limits amount of resolved' do
-        before do
-          3.times { add_resolved_failure }
-          3.times { add_resolved_error }
-        end
-
-        it 'returns 2 of each resolved category' do
-          expect(subject[:summary]).to include(total: 6, resolved: 6, failed: 0, errored: 0)
-          expect(subject[:new_failures].count).to eq(0)
-          expect(subject[:new_errors].count).to eq(0)
-          expect(subject[:existing_failures].count).to eq(0)
-          expect(subject[:existing_errors].count).to eq(0)
-          expect(subject[:resolved_failures].count).to eq(2)
-          expect(subject[:resolved_errors].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_new_error
-        error_case = create_test_case_rspec_error(SecureRandom.hex)
-        head_suite.add_test_case(error_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_existing_error
-        error_case = create_test_case_rspec_error(SecureRandom.hex)
-        base_suite.add_test_case(error_case)
-        head_suite.add_test_case(error_case)
-      end
-
-      def add_resolved_failure
-        case_name = SecureRandom.hex
-        failed_case = create_test_case_java_failed(case_name)
-        success_case = create_test_case_java_success(case_name)
-        base_suite.add_test_case(failed_case)
-        head_suite.add_test_case(success_case)
-      end
-
-      def add_resolved_error
-        case_name = SecureRandom.hex
-        error_case = create_test_case_java_error(case_name)
-        success_case = create_test_case_java_success(case_name)
-        base_suite.add_test_case(error_case)
-        head_suite.add_test_case(success_case)
-      end
-    end
   end
 end

spec/services/ci/compare_test_reports_service_spec.rb

@@ -7,15 +7,15 @@ RSpec.describe Ci::CompareTestReportsService do
   let(:project) { create(:project, :repository) }
 
   describe '#execute' do
-    subject { service.execute(base_pipeline, head_pipeline) }
+    subject(:comparison) { service.execute(base_pipeline, head_pipeline) }
 
     context 'when head pipeline has test reports' do
       let!(:base_pipeline) { nil }
       let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
 
       it 'returns status and data' do
-        expect(subject[:status]).to eq(:parsed)
-        expect(subject[:data]).to match_schema('entities/test_reports_comparer')
+        expect(comparison[:status]).to eq(:parsed)
+        expect(comparison[:data]).to match_schema('entities/test_reports_comparer')
       end
     end
 
@@ -24,8 +24,8 @@ RSpec.describe Ci::CompareTestReportsService do
       let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
 
       it 'returns status and data' do
-        expect(subject[:status]).to eq(:parsed)
-        expect(subject[:data]).to match_schema('entities/test_reports_comparer')
+        expect(comparison[:status]).to eq(:parsed)
+        expect(comparison[:data]).to match_schema('entities/test_reports_comparer')
       end
     end
 
@@ -39,9 +39,44 @@ RSpec.describe Ci::CompareTestReportsService do
       end
 
       it 'returns a parsed TestReports success status and failure on the individual suite' do
-        expect(subject[:status]).to eq(:parsed)
-        expect(subject.dig(:data, 'status')).to eq('success')
-        expect(subject.dig(:data, 'suites', 0, 'status') ).to eq('error')
+        expect(comparison[:status]).to eq(:parsed)
+        expect(comparison.dig(:data, 'status')).to eq('success')
+        expect(comparison.dig(:data, 'suites', 0, 'status') ).to eq('error')
+      end
+    end
+
+    context 'test failure history' do
+      let!(:base_pipeline) { nil }
+      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
+        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 3 failures.
+        # This service will create 1 test case failure record for each.
+        Ci::TestCasesService.new.execute(build)
+      end
+
+      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