Merge branch 'improve-junit-support-be' into 'master'

Improve JUnit test reports in merge request widgets

Closes #49966

See merge request gitlab-org/gitlab-ce!21039

5 files changed, 66 insertions, 32 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index eaf4434f913..1b069fe507b 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -102,10 +102,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   def test_reports
     result = @merge_request.compare_test_reports
 
-    Gitlab::PollingInterval.set_header(response, interval: 10_000)
-
     case result[:status]
     when :parsing
+      Gitlab::PollingInterval.set_header(response, interval: 3000)
+
       render json: '', status: :no_content
     when :parsed
       render json: result[:data].to_json, status: :ok
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 75dfa00d12e..e4aed76f611 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -606,12 +606,12 @@ module Ci
     end
 
     def has_test_reports?
-      complete? && builds.with_test_reports.any?
+      complete? && builds.latest.with_test_reports.any?
     end
 
     def test_reports
       Gitlab::Ci::Reports::TestReports.new.tap do |test_reports|
-        builds.with_test_reports.each do |build|
+        builds.latest.with_test_reports.each do |build|
           build.collect_test_reports!(test_reports)
         end
       end
diff --git a/app/models/concerns/reactive_caching.rb b/app/models/concerns/reactive_caching.rb
index 9155d82d567..65cc7a751f9 100644
--- a/app/models/concerns/reactive_caching.rb
+++ b/app/models/concerns/reactive_caching.rb
@@ -42,6 +42,8 @@
 module ReactiveCaching
   extend ActiveSupport::Concern
 
+  InvalidateReactiveCache = Class.new(StandardError)
+
   included do
     class_attribute :reactive_cache_lease_timeout
 
@@ -63,15 +65,19 @@ module ReactiveCaching
     end
 
     def with_reactive_cache(*args, &blk)
-      bootstrap = !within_reactive_cache_lifetime?(*args)
-      Rails.cache.write(alive_reactive_cache_key(*args), true, expires_in: self.class.reactive_cache_lifetime)
+      unless within_reactive_cache_lifetime?(*args)
+        refresh_reactive_cache!(*args)
+        return nil
+      end
 
-      if bootstrap
-        ReactiveCachingWorker.perform_async(self.class, id, *args)
-        nil
-      else
+      keep_alive_reactive_cache!(*args)
+
+      begin
         data = Rails.cache.read(full_reactive_cache_key(*args))
         yield data if data.present?
+      rescue InvalidateReactiveCache
+        refresh_reactive_cache!(*args)
+        nil
       end
     end
 
@@ -96,6 +102,16 @@ module ReactiveCaching
 
     private
 
+    def refresh_reactive_cache!(*args)
+      clear_reactive_cache!(*args)
+      keep_alive_reactive_cache!(*args)
+      ReactiveCachingWorker.perform_async(self.class, id, *args)
+    end
+
+    def keep_alive_reactive_cache!(*args)
+      Rails.cache.write(alive_reactive_cache_key(*args), true, expires_in: self.class.reactive_cache_lifetime)
+    end
+
     def full_reactive_cache_key(*qualifiers)
       prefix = self.class.reactive_cache_key
       prefix = prefix.call(self) if prefix.respond_to?(:call)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 9b3e2d4446d..396647a14ae 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -16,8 +16,8 @@ class MergeRequest < ActiveRecord::Base
   include ReactiveCaching
 
   self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
-  self.reactive_cache_refresh_interval = 1.hour
-  self.reactive_cache_lifetime = 1.hour
+  self.reactive_cache_refresh_interval = 10.minutes
+  self.reactive_cache_lifetime = 10.minutes
 
   ignore_column :locked_at,
                 :ref_fetched,
@@ -1041,16 +1041,21 @@ class MergeRequest < ActiveRecord::Base
       return { status: :error, status_reason: 'This merge request does not have test reports' }
     end
 
-    with_reactive_cache(
-      :compare_test_results,
-      base_pipeline&.iid,
-      actual_head_pipeline.iid) { |data| data } || { status: :parsing }
+    with_reactive_cache(:compare_test_results) do |data|
+      unless Ci::CompareTestReportsService.new(project)
+        .latest?(base_pipeline, actual_head_pipeline, data)
+        raise InvalidateReactiveCache
+      end
+
+      data
+    end || { status: :parsing }
   end
 
   def calculate_reactive_cache(identifier, *args)
     case identifier.to_sym
     when :compare_test_results
-      Ci::CompareTestReportsService.new(project).execute(*args)
+      Ci::CompareTestReportsService.new(project).execute(
+        base_pipeline, actual_head_pipeline)
     else
       raise NotImplementedError, "Unknown identifier: #{identifier}"
     end
diff --git a/app/services/ci/compare_test_reports_service.rb b/app/services/ci/compare_test_reports_service.rb
index 7a112211d94..ec25e934a27 100644
--- a/app/services/ci/compare_test_reports_service.rb
+++ b/app/services/ci/compare_test_reports_service.rb
@@ -2,23 +2,36 @@
 
 module Ci
   class CompareTestReportsService < ::BaseService
-    def execute(base_pipeline_iid, head_pipeline_iid)
-      base_pipeline = project.pipelines.find_by_iid(base_pipeline_iid) if base_pipeline_iid
-      head_pipeline = project.pipelines.find_by_iid(head_pipeline_iid)
+    def execute(base_pipeline, head_pipeline)
+      comparer = Gitlab::Ci::Reports::TestReportsComparer
+        .new(base_pipeline&.test_reports, head_pipeline.test_reports)
 
-      begin
-        comparer = Gitlab::Ci::Reports::TestReportsComparer
-          .new(base_pipeline&.test_reports, head_pipeline.test_reports)
+      {
+        status: :parsed,
+        key: key(base_pipeline, head_pipeline),
+        data: TestReportsComparerSerializer
+          .new(project: project)
+          .represent(comparer).as_json
+      }
+    rescue => e
+      {
+        status: :error,
+        key: key(base_pipeline, head_pipeline),
+        status_reason: e.message
+      }
+    end
+
+    def latest?(base_pipeline, head_pipeline, data)
+      data&.fetch(:key, nil) == key(base_pipeline, head_pipeline)
+    end
+
+    private
 
-        {
-          status: :parsed,
-          data: TestReportsComparerSerializer
-            .new(project: project)
-            .represent(comparer).as_json
-        }
-      rescue => e
-        { status: :error, status_reason: e.message }
-      end
+    def key(base_pipeline, head_pipeline)
+      [
+        base_pipeline&.id, base_pipeline&.updated_at,
+        head_pipeline&.id, head_pipeline&.updated_at
+      ]
     end
   end
 end