Refactoring test_results to test_reports

19 files changed, 118 insertions, 153 deletions

diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index bb78f3aed26..672e5280b5e 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -108,7 +108,7 @@ export default class MergeRequestStore {
     this.isPipelineBlocked = pipelineStatus ? pipelineStatus.group === 'manual' : false;
     this.ciStatusFaviconPath = pipelineStatus ? pipelineStatus.favicon : null;
 
-    this.testResultsPath = data.test_results_path;
+    this.testResultsPath = data.test_reports_path;
 
     this.setState(data);
   }
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 46d9911546c..a6f7046cb04 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -99,12 +99,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     }
   end
 
-  def test_results
-    test_results = @merge_request.compared_test_results
+  def test_reports
+    test_reports = @merge_request.compared_test_reports
 
-    render json: TestResultsComparerSerializer
+    render json: TestReportsComparerSerializer
       .new(project: @project, current_user: @current_user)
-      .represent(test_results)
+      .represent(test_reports)
   end
 
   def edit
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 1b749dd67d0..fcfc3120757 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -614,12 +614,13 @@ module Ci
       running? && runner_session_url.present?
     end
 
-    def collect_test_results(test_results)
-      return unless job_artifacts_junit
-
-      test_results.get_suite(group_name).tap do |test_suite|
-        job_artifacts_junit.each_blob do |blob, name|
-          Gitlab::Ci::Parsers::JunitParser.new(blob).parse!(test_suite)
+    def collect_test_reports(test_reports)
+      test_reports.get_suite(group_name).tap do |test_suite|
+        job_artifacts.test_reports.each do |report_artifact|
+          report_artifact.each_blob do |blob, name|
+            "Gitlab::Ci::Parsers::#{report_artifact.file_type.capitalize}Parser".constantize
+              .new(blob).parse!(test_suite)
+          end
         end
       end
     end
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 25393221005..b2455b167b0 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -603,14 +603,14 @@ module Ci
       @latest_builds_with_artifacts ||= builds.latest.with_artifacts_archive.to_a
     end
 
-    def has_test_results?
+    def has_test_reports?
       complete? && builds.with_test_reports.any?
     end
 
-    def test_results
-      Gitlab::Ci::Reports::TestResults.new.tap do |test_results|
+    def test_reports
+      Gitlab::Ci::Reports::TestReports.new.tap do |test_reports|
         builds.with_test_reports.includes(:job_artifacts_junit).each do |build|
-          build.collect_test_results(test_results)
+          build.collect_test_reports(test_reports)
         end
       end
     end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 5a3ad3f60ce..d2590fb516b 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -1009,14 +1009,14 @@ class MergeRequest < ActiveRecord::Base
       .order(id: :desc)
   end
 
-  def has_test_results?
-    actual_head_pipeline&.has_test_results?
+  def has_test_reports?
+    actual_head_pipeline&.has_test_reports?
   end
 
-  def compared_test_results
+  def compared_test_reports
     return unless actual_head_pipeline
 
-    Gitlab::Ci::Reports::TestResultsComparer.new(base_pipeline&.test_results, actual_head_pipeline&.test_results)
+    Gitlab::Ci::Reports::TestReportsComparer.new(base_pipeline&.test_reports, actual_head_pipeline&.test_reports)
   end
 
   def all_commits
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index fd9f9dcc9af..36197d2bd6a 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -225,9 +225,9 @@ class MergeRequestWidgetEntity < IssuableEntity
     end
   end
 
-  expose :test_results_path do |merge_request|
-    if merge_request.has_test_results?
-      test_results_project_merge_request_path(merge_request.project, merge_request, format: :json)
+  expose :test_reports_path do |merge_request|
+    if merge_request.has_test_reports?
+      test_reports_project_merge_request_path(merge_request.project, merge_request, format: :json)
     end
   end
 
diff --git a/app/serializers/test_case_entity.rb b/app/serializers/test_case_entity.rb
index 8213b6d4179..a8d7a4bf61e 100644
--- a/app/serializers/test_case_entity.rb
+++ b/app/serializers/test_case_entity.rb
@@ -1,4 +1,5 @@
 class TestCaseEntity < Grape::Entity
+  expose :result
   expose :name
   expose :time, as: :execution_time
   expose :failure_reason, as: :system_output
diff --git a/app/serializers/test_results_comparer_entity.rb b/app/serializers/test_reports_comparer_entity.rb
index 395dc689dda..7791bcf45dc 100644
--- a/app/serializers/test_results_comparer_entity.rb
+++ b/app/serializers/test_reports_comparer_entity.rb
@@ -1,4 +1,4 @@
-class TestResultsComparerEntity < Grape::Entity
+class TestReportsComparerEntity < Grape::Entity
   expose :summary do
     expose :total_count, as: :total
     expose :resolved_count, as: :resolved
diff --git a/app/serializers/test_reports_comparer_serializer.rb b/app/serializers/test_reports_comparer_serializer.rb
new file mode 100644
index 00000000000..a739858efb2
--- /dev/null
+++ b/app/serializers/test_reports_comparer_serializer.rb
@@ -0,0 +1,3 @@
+class TestReportsComparerSerializer < BaseSerializer
+  entity TestReportsComparerEntity
+end
diff --git a/app/serializers/test_results_comparer_serializer.rb b/app/serializers/test_results_comparer_serializer.rb
deleted file mode 100644
index 53983d63b0d..00000000000
--- a/app/serializers/test_results_comparer_serializer.rb
+++ /dev/null
@@ -1,3 +0,0 @@
-class TestResultsComparerSerializer < BaseSerializer
-  entity TestResultsComparerEntity
-end
diff --git a/config/routes/project.rb b/config/routes/project.rb
index be0c1feca62..fce620c4ac1 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -109,7 +109,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           post :assign_related_issues
           get :discussions, format: :json
           post :rebase
-          get :test_results
+          get :test_reports
 
           scope constraints: { format: nil }, action: :show do
             get :commits, defaults: { tab: 'commits' }
diff --git a/lib/gitlab/ci/parsers/junit_parser.rb b/lib/gitlab/ci/parsers/junit_parser.rb
index 0ead2121aab..08c10e89c6c 100644
--- a/lib/gitlab/ci/parsers/junit_parser.rb
+++ b/lib/gitlab/ci/parsers/junit_parser.rb
@@ -12,11 +12,12 @@ module Gitlab
           each_suite do |testcases, _|
             testcases.each do |testcase|
               test_case = create_test_case(testcase)
-              test_suite.add_result(test_case)
+              test_suite.add_test_case(test_case)
             end
           end
-        rescue
+        rescue => e
           Rails.logger.error "Failed to parse Junit file" # Since xml_data is user-generated contents, parser could fail if they include corrupted-data
+          raise e # TODO: Remove
         end
 
         private
@@ -31,10 +32,10 @@ module Gitlab
 
         def create_test_case(data)
           if data['failure']
-            status = ::Gitlab::Ci::Reports::TestCase::FAILED
+            result = ::Gitlab::Ci::Reports::TestCase::RESULT_FAILURE
             failure_reason = data['failure']
           else
-            status = ::Gitlab::Ci::Reports::TestCase::SUCCESS
+            result = ::Gitlab::Ci::Reports::TestCase::RESULT_PASS
             failure_reason = nil
           end
 
@@ -43,7 +44,7 @@ module Gitlab
             name: data['name'],
             file: data['file'],
             time: data['time'],
-            status: status,
+            result: result,
             failure_reason: failure_reason
           )
         end
diff --git a/lib/gitlab/ci/reports/test_case.rb b/lib/gitlab/ci/reports/test_case.rb
index 6c1d6ad95d9..dfc0dcfde35 100644
--- a/lib/gitlab/ci/reports/test_case.rb
+++ b/lib/gitlab/ci/reports/test_case.rb
@@ -2,22 +2,22 @@ module Gitlab
   module Ci
     module Reports
       class TestCase
-        STATUSES = %w(success failed skipped error)
-        SUCCESS = :success
-        FAILED = :failed
-        SKIPPED = :skipped
-        ERROR = :error
+        RESULT_PASS = 'pass'.freeze
+        RESULT_FAILURE = 'failure'.freeze
+        RESULT_SKIPPED = 'skipped'.freeze
+        RESULT_ERROR = 'error'.freeze
+        RESULT_TYPES = [RESULT_PASS, RESULT_FAILURE, RESULT_SKIPPED, RESULT_ERROR].freeze
 
-        attr_reader :name, :classname, :file, :time, :status, :failure_reason, :key
+        attr_reader :name, :classname, :file, :time, :result, :failure_reason, :key
 
-        def initialize(name:, classname:, file:, time:, status:, failure_reason:)
+        def initialize(name:, classname:, file:, time:, result:, failure_reason:)
           raise 'Insufficient data to generate a unique key' unless classname.present? && name.present?
 
           @name = name
           @classname = classname
           @file = file
-          @time = time
-          @status = status
+          @time = time.to_f
+          @result = result
           @failure_reason = failure_reason
           @key = sanitize_key_name("#{classname}_#{name}")
         end
diff --git a/lib/gitlab/ci/reports/test_reports.rb b/lib/gitlab/ci/reports/test_reports.rb
new file mode 100644
index 00000000000..f77a772c53f
--- /dev/null
+++ b/lib/gitlab/ci/reports/test_reports.rb
@@ -0,0 +1,25 @@
+module Gitlab
+  module Ci
+    module Reports
+      class TestReports < Hash
+        def get_suite(suite_name)
+          self[suite_name] ||= TestSuite.new(suite_name)
+        end
+
+        def total_time
+          self.values.sum(&:total_time)
+        end
+
+        def total_count
+          self.values.sum(&:total_count)
+        end
+
+        TestCase::RESULT_TYPES.each do |result_type|
+          define_method("#{result_type}_count") do
+            self.values.sum { |suite| suite.send("#{result_type}_count") }
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/reports/test_reports_comparer.rb b/lib/gitlab/ci/reports/test_reports_comparer.rb
new file mode 100644
index 00000000000..0ac0b9e405e
--- /dev/null
+++ b/lib/gitlab/ci/reports/test_reports_comparer.rb
@@ -0,0 +1,30 @@
+module Gitlab
+  module Ci
+    module Reports
+      class TestReportsComparer
+        include Gitlab::Utils::StrongMemoize
+
+        attr_reader :base_reports, :head_reports
+
+        def initialize(base_reports, head_reports)
+          @base_reports = base_reports || TestReports.new
+          @head_reports = head_reports
+        end
+
+        def suites
+          strong_memoize(:suites) do
+            head_reports.map do |name, test_suite|
+              TestSuiteComparer.new(name, base_reports[name], test_suite)
+            end
+          end
+        end
+
+        %w(total_count resolved_count failed_count).each do |method|
+          define_method(method) do
+            suites.sum { |suite| suite.public_send(method) }
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/reports/test_results.rb b/lib/gitlab/ci/reports/test_results.rb
deleted file mode 100644
index 442f033ea0a..00000000000
--- a/lib/gitlab/ci/reports/test_results.rb
+++ /dev/null
@@ -1,55 +0,0 @@
-module Gitlab
-  module Ci
-    module Reports
-      class TestResults
-        attr_reader :suites
-
-        def initialize
-          @suites = {}
-        end
-
-        def get_suite(suite_name)
-          @suites[suite_name] ||= TestSuite.new(suite_name)
-        end
-
-        def total_time
-          suites.values.sum(&:total_time)
-        end
-
-        def total_count
-          suites.values.sum(&:total_count)
-        end
-
-        TestCase::STATUSES.each do |result_type|
-          define_method("#{result_type}_count") do
-            suites.values.sum { |suite| suite.send("#{result_type}_count") }
-          end
-        end
-
-        private
-
-        def compare_failed_tests(base_test_results)
-          head = self.failed_tests.map(&:key)
-          base = base_test_results.failed_tests.map(&:key)
-
-          new_failures = head - base
-          resolved_failures = base - head
-          existing_failures = head - new_failures
-
-          # TODO: Return TestCase instead of returning keys
-          {
-            new: new_failures,
-            resolved: resolved_failures,
-            existing: existing_failures
-          }
-        end
-
-        def failed_tests
-          suites.keys.map do |suite|
-            suites[suite][TestCase::RESULT_FAILED]
-          end.flatten.compact
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/reports/test_results_comparer.rb b/lib/gitlab/ci/reports/test_results_comparer.rb
deleted file mode 100644
index 752bb9f9914..00000000000
--- a/lib/gitlab/ci/reports/test_results_comparer.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-module Gitlab
-  module Ci
-    module Reports
-      class TestResultsComparer
-        include Gitlab::Utils::StrongMemoize
-
-        attr_reader :base_results, :head_results
-
-        def initialize(base_results, head_results)
-          @base_results = base_results || TestResults.new
-          @head_results = head_results
-        end
-
-        def suites
-          strong_memoize(:suites) do
-            head_results.suites.map do |name, test_suite|
-              TestSuiteComparer.new(name, base_results.suites[name], test_suite)
-            end
-          end
-        end
-
-        %w(total_count resolved_count failed_count).each do |method|
-          define_method(method) do
-            strong_memoize(method) do
-              suites.sum { |suite| suite.public_send(method) }
-            end
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/reports/test_suite.rb b/lib/gitlab/ci/reports/test_suite.rb
index e51f526a489..64e351f5c1b 100644
--- a/lib/gitlab/ci/reports/test_suite.rb
+++ b/lib/gitlab/ci/reports/test_suite.rb
@@ -3,33 +3,33 @@ module Gitlab
     module Reports
       class TestSuite
         attr_reader :name
-        attr_reader :test_statuses
+        attr_reader :test_cases
         attr_reader :total_time
 
         def initialize(name = nil)
           @name = name
-          @test_statuses = {}
+          @test_cases = {}
           @total_time = 0.0
         end
 
-        TestCase::STATUSES.each do |result_type|
+        TestCase::RESULT_TYPES.each do |result_type|
           define_method("#{result_type}") do
-            test_statuses[result_type.to_sym] || {}
+            test_cases[result_type] || {}
           end
 
           define_method("#{result_type}_count") do
-            test_statuses[result_type.to_sym]&.length.to_i
+            test_cases[result_type]&.length.to_i
           end
         end
 
-        def add_result(test_case)
-          @test_statuses[test_case.status] ||= {}
-          @test_statuses[test_case.status][test_case.key] = test_case
-          @total_time += test_case.time.to_f
+        def add_test_case(test_case)
+          @test_cases[test_case.result] ||= {}
+          @test_cases[test_case.result][test_case.key] = test_case
+          @total_time += test_case.time
         end
 
         def total_count
-          test_statuses.values.sum(&:count)
+          test_cases.values.sum(&:count)
         end
       end
     end
diff --git a/lib/gitlab/ci/reports/test_suite_comparer.rb b/lib/gitlab/ci/reports/test_suite_comparer.rb
index 64d29d3a5fe..0437f338983 100644
--- a/lib/gitlab/ci/reports/test_suite_comparer.rb
+++ b/lib/gitlab/ci/reports/test_suite_comparer.rb
@@ -14,44 +14,38 @@ module Gitlab
 
         def new_failures
           strong_memoize(:new_failures) do
-            head_suite.failed.reject do |key, _|
-              base_suite.failed.include?(key)
+            head_suite.failure.reject do |key, _|
+              base_suite.failure.include?(key)
             end.values
           end
         end
 
         def existing_failures
           strong_memoize(:existing_failures) do
-            head_suite.failed.select do |key, _|
-              base_suite.failed.include?(key)
+            head_suite.failure.select do |key, _|
+              base_suite.failure.include?(key)
             end.values
           end
         end
 
         def resolved_failures
           strong_memoize(:resolved_failures) do
-            head_suite.success.select do |key, _|
-              base_suite.failed.include?(key)
+            head_suite.pass.select do |key, _|
+              base_suite.failure.include?(key)
             end.values
           end
         end
 
         def total_count
-          strong_memoize(:total_count) do
-            head_suite.total_count
-          end
+          head_suite.total_count
         end
 
         def resolved_count
-          strong_memoize(:resolved_count) do
-            resolved_failures.count
-          end
+          resolved_failures.count
         end
 
         def failed_count
-          strong_memoize(:failed) do
-            new_failures.count + existing_failures.count
-          end
+          new_failures.count + existing_failures.count
         end
       end
     end