diff options
author | Shinya Maeda <shinya@gitlab.com> | 2018-07-23 16:58:56 +0900 |
---|---|---|
committer | samdbeckham <sbeckham@gitlab.com> | 2018-07-25 12:41:56 +0100 |
commit | b20da289fb6c45466bf47a09ac6c33625806936b (patch) | |
tree | c8f5a448e4939f485cf842ba6fb0b49f09390359 | |
parent | f1cef58465f5936a74c0cb23167b5d4a51ecc46c (diff) | |
download | gitlab-ce-b20da289fb6c45466bf47a09ac6c33625806936b.tar.gz |
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 37499ba975d..5222d7c7c89 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -227,9 +227,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 |