Commit e6226e8c authored by Gilbert Roulot's avatar Gilbert Roulot Committed by Kamil Trzciński

Generalise test compare service

It adds a base class for CompareTestReportsService
containing common code with CompareLicenseManagementReportsService
which is present in GitLab Enterprise Edition.
parent 85f430cb
Pipeline #39704540 passed with stages
in 109 minutes and 9 seconds
......@@ -742,7 +742,7 @@ module Ci
def collect_test_reports!(test_reports)
test_reports.get_suite(group_name).tap do |test_suite|
each_report(Ci::JobArtifact::TEST_REPORT_FILE_TYPES) do |file_type, blob|
Gitlab::Ci::Parsers::Test.fabricate!(file_type).parse!(blob, test_suite)
Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite)
end
end
end
......
......@@ -1118,14 +1118,17 @@ class MergeRequest < ActiveRecord::Base
end
end
# rubocop: disable CodeReuse/ServiceClass
def compare_test_reports
unless has_test_reports?
return { status: :error, status_reason: 'This merge request does not have test reports' }
end
with_reactive_cache(:compare_test_results) do |data|
unless Ci::CompareTestReportsService.new(project)
compare_reports(Ci::CompareTestReportsService)
end
def compare_reports(service_class)
with_reactive_cache(service_class.name) do |data|
unless service_class.new(project)
.latest?(base_pipeline, actual_head_pipeline, data)
raise InvalidateReactiveCache
end
......@@ -1133,19 +1136,14 @@ class MergeRequest < ActiveRecord::Base
data
end || { status: :parsing }
end
# rubocop: enable CodeReuse/ServiceClass
# rubocop: disable CodeReuse/ServiceClass
def calculate_reactive_cache(identifier, *args)
case identifier.to_sym
when :compare_test_results
Ci::CompareTestReportsService.new(project).execute(
base_pipeline, actual_head_pipeline)
else
raise NotImplementedError, "Unknown identifier: #{identifier}"
end
service_class = identifier.constantize
raise NameError, service_class unless service_class < Ci::CompareReportsBaseService
service_class.new(project).execute(base_pipeline, actual_head_pipeline)
end
# rubocop: enable CodeReuse/ServiceClass
def all_commits
# MySQL doesn't support LIMIT in a subquery.
......
# frozen_string_literal: true
module Ci
class CompareReportsBaseService < ::BaseService
def execute(base_pipeline, head_pipeline)
comparer = comparer_class.new(get_report(base_pipeline), get_report(head_pipeline))
{
status: :parsed,
key: key(base_pipeline, head_pipeline),
data: serializer_class
.new(project: project)
.represent(comparer).as_json
}
rescue Gitlab::Ci::Parsers::ParserError => 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
def key(base_pipeline, head_pipeline)
[
base_pipeline&.id, base_pipeline&.updated_at,
head_pipeline&.id, head_pipeline&.updated_at
]
end
def comparer_class
raise NotImplementedError
end
def serializer_class
raise NotImplementedError
end
def get_report(pipeline)
raise NotImplementedError
end
end
end
# frozen_string_literal: true
module Ci
class CompareTestReportsService < ::BaseService
def execute(base_pipeline, head_pipeline)
# rubocop: disable CodeReuse/Serializer
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
}
# rubocop: enable CodeReuse/Serializer
class CompareTestReportsService < CompareReportsBaseService
def comparer_class
Gitlab::Ci::Reports::TestReportsComparer
end
def latest?(base_pipeline, head_pipeline, data)
data&.fetch(:key, nil) == key(base_pipeline, head_pipeline)
def serializer_class
TestReportsComparerSerializer
end
private
def key(base_pipeline, head_pipeline)
[
base_pipeline&.id, base_pipeline&.updated_at,
head_pipeline&.id, head_pipeline&.updated_at
]
def get_report(pipeline)
pipeline&.test_reports
end
end
end
......@@ -3,18 +3,18 @@
module Gitlab
module Ci
module Parsers
module Test
ParserNotFoundError = Class.new(StandardError)
ParserNotFoundError = Class.new(ParserError)
PARSERS = {
def self.parsers
{
junit: ::Gitlab::Ci::Parsers::Test::Junit
}.freeze
}
end
def self.fabricate!(file_type)
PARSERS.fetch(file_type.to_sym).new
rescue KeyError
raise ParserNotFoundError, "Cannot find any parser matching file type '#{file_type}'"
end
def self.fabricate!(file_type)
parsers.fetch(file_type.to_sym).new
rescue KeyError
raise ParserNotFoundError, "Cannot find any parser matching file type '#{file_type}'"
end
end
end
......
# frozen_string_literal: true
module Gitlab
module Ci
module Parsers
ParserError = Class.new(StandardError)
end
end
end
......@@ -5,7 +5,7 @@ module Gitlab
module Parsers
module Test
class Junit
JunitParserError = Class.new(StandardError)
JunitParserError = Class.new(Gitlab::Ci::Parsers::ParserError)
def parse!(xml_data, test_suite)
root = Hash.from_xml(xml_data)
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Ci::Parsers::Test do
describe Gitlab::Ci::Parsers do
describe '.fabricate!' do
subject { described_class.fabricate!(file_type) }
......@@ -8,7 +10,7 @@ describe Gitlab::Ci::Parsers::Test do
let(:file_type) { 'junit' }
it 'fabricates the class' do
is_expected.to be_a(described_class::Junit)
is_expected.to be_a(described_class::Test::Junit)
end
end
......@@ -16,7 +18,7 @@ describe Gitlab::Ci::Parsers::Test do
let(:file_type) { 'undefined' }
it 'raises an error' do
expect { subject }.to raise_error(Gitlab::Ci::Parsers::Test::ParserNotFoundError)
expect { subject }.to raise_error(Gitlab::Ci::Parsers::ParserNotFoundError)
end
end
end
......
......@@ -1339,6 +1339,30 @@ describe MergeRequest do
end
end
describe '#calculate_reactive_cache' do
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, source_project: project) }
subject { merge_request.calculate_reactive_cache(service_class_name) }
context 'when given an unknown service class name' do
let(:service_class_name) { 'Integer' }
it 'raises a NameError exception' do
expect { subject }.to raise_error(NameError, service_class_name)
end
end
context 'when given a known service class name' do
let(:service_class_name) { 'Ci::CompareTestReportsService' }
it 'does not raises a NameError exception' do
allow_any_instance_of(service_class_name.constantize).to receive(:execute).and_return(nil)
expect { subject }.not_to raise_error(NameError)
end
end
end
describe '#compare_test_reports' do
subject { merge_request.compare_test_reports }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment