Refactor test suites limit out of the serializer

We move out the test suite limiting out of the serializer and move it to
the compare service so that the same limit will be used by the test
failure history query.

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

@@ -14,15 +14,23 @@ module Ci
       pipeline&.test_reports
     end
 
-    def build_comparer(base_pipeline, head_pipeline)
-      base_report = get_report(base_pipeline)
-      head_report = get_report(head_pipeline)
-
-      # We need to load the test failure history for the head report because we display
+    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
-      ::Gitlab::Ci::Reports::TestReportFailureHistory.new(head_report, project).load!
+      super.tap do |test_reports_comparer|
+        ::Gitlab::Ci::Reports::TestFailureHistory.new(failed_test_cases(test_reports_comparer), project).load!
+      end
+    end
 
-      comparer_class.new(base_report, head_report)
+    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
+      end
     end
   end
 end

lib/gitlab/ci/reports/test_report_failure_history.rb → lib/gitlab/ci/reports/test_failure_history.rb

@@ -3,11 +3,11 @@
 module Gitlab
   module Ci
     module Reports
-      class TestReportFailureHistory
+      class TestFailureHistory
         include Gitlab::Utils::StrongMemoize
 
-        def initialize(report, project)
-          @report = report
+        def initialize(failed_test_cases, project)
+          @failed_test_cases = build_map(failed_test_cases)
           @project = project
         end
 
@@ -21,7 +21,7 @@ module Gitlab
 
         private
 
-        attr_reader :report, :project
+        attr_reader :report, :project, :failed_test_cases
 
         def recent_failures_count
           ::Ci::TestCaseFailure.recent_failures_count(
@@ -30,9 +30,11 @@ module Gitlab
           )
         end
 
-        def failed_test_cases
-          strong_memoize(:failed_test_cases) do
-            report.failed_test_cases
+        def build_map(test_cases)
+          {}.tap do |hash|
+            test_cases.each do |test_case|
+              hash[test_case.key] = test_case
+            end
           end
         end
       end

lib/gitlab/ci/reports/test_reports.rb

@@ -52,14 +52,6 @@ module Gitlab
             test_suites.values.sum { |suite| suite.public_send("#{status_type}_count") } # rubocop:disable GitlabSecurity/PublicSend
             # rubocop: enable CodeReuse/ActiveRecord
           end
-
-          define_method("#{status_type}_test_cases") do
-            {}.tap do |hash|
-              test_suites.values.each do |test_suite|
-                hash.merge!(test_suite.public_send(status_type)) # rubocop:disable GitlabSecurity/PublicSend
-              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/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/lib/gitlab/ci/reports/test_report_failure_history_spec.rb → spec/lib/gitlab/ci/reports/test_failure_history_spec.rb

@@ -2,21 +2,17 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Ci::Reports::TestReportFailureHistory, :aggregate_failures do
+RSpec.describe Gitlab::Ci::Reports::TestFailureHistory, :aggregate_failures do
   include TestReportsHelper
 
   describe '#load!' do
     let_it_be(:project) { create(:project) }
-    let(:test_reports) { Gitlab::Ci::Reports::TestReports.new }
     let(:failed_rspec) { create_test_case_rspec_failed }
     let(:failed_java) { create_test_case_java_failed }
 
-    subject(:load_history) { described_class.new(test_reports, project).load! }
+    subject(:load_history) { described_class.new([failed_rspec, failed_java], project).load! }
 
     before do
-      test_reports.get_suite('rspec').add_test_case(failed_rspec)
-      test_reports.get_suite('java').add_test_case(failed_java)
-
       allow(Ci::TestCaseFailure)
         .to receive(:recent_failures_count)
         .with(project: project, test_case_keys: [failed_rspec.key, failed_java.key])

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

@@ -185,30 +185,5 @@ RSpec.describe Gitlab::Ci::Reports::TestReports do
         end
       end
     end
-
-    describe "##{status_type}_test_cases" do
-      subject { test_reports.public_send("#{status_type}_test_cases") }
-
-      context "when #{status_type} test case exists" do
-        it 'returns the test cases' do
-          rspec_case = public_send("create_test_case_rspec_#{status_type}")
-          java_case = public_send("create_test_case_java_#{status_type}")
-
-          test_reports.get_suite('rspec').add_test_case(rspec_case)
-          test_reports.get_suite('junit').add_test_case(java_case)
-
-          is_expected.to eq(
-            rspec_case.key => rspec_case,
-            java_case.key => java_case
-          )
-        end
-      end
-
-      context "when #{status_type} test case do not exist" do
-        it 'returns nothing' do
-          is_expected.to eq({})
-        end
-      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/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,9 @@ 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
 
@@ -50,7 +50,7 @@ RSpec.describe Ci::CompareTestReportsService do
       let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
 
       let(:recent_failures_per_test_case) do
-        subject.dig(:data, 'suites', 0, 'new_failures').map { |f| f['recent_failures'] }
+        comparison.dig(:data, 'suites', 0, 'new_failures').map { |f| f['recent_failures'] }
       end
 
       # Create test case failure records based on the head pipeline build
@@ -64,7 +64,7 @@ RSpec.describe Ci::CompareTestReportsService do
       end
 
       it 'loads on the report', :aggregate_failures do
-        expect(subject[:data]).to match_schema('entities/test_reports_comparer')
+        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' }