Commit 04d83aec authored by Kamil Trzciński's avatar Kamil Trzciński

Refactor `performance:` to use new reports feature

parent a5f8eab9
...@@ -8,7 +8,6 @@ module EE ...@@ -8,7 +8,6 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
LICENSE_MANAGEMENT_FILE = 'gl-license-management-report.json'.freeze LICENSE_MANAGEMENT_FILE = 'gl-license-management-report.json'.freeze
PERFORMANCE_FILE = 'performance.json'.freeze
LICENSED_PARSER_FEATURES = { LICENSED_PARSER_FEATURES = {
sast: :sast sast: :sast
...@@ -34,11 +33,6 @@ module EE ...@@ -34,11 +33,6 @@ module EE
::Gitlab::Database::LoadBalancing::Sticking.stick(:build, id) ::Gitlab::Database::LoadBalancing::Sticking.stick(:build, id)
end end
def has_performance_json?
name_in?(%w[performance deploy]) &&
has_artifact?(PERFORMANCE_FILE)
end
def has_license_management_json? def has_license_management_json?
name_in?('license_management') && name_in?('license_management') &&
has_artifact?(LICENSE_MANAGEMENT_FILE) has_artifact?(LICENSE_MANAGEMENT_FILE)
......
...@@ -27,7 +27,8 @@ module EE ...@@ -27,7 +27,8 @@ module EE
sast: %i[sast], sast: %i[sast],
dependency_scanning: %i[dependency_scanning], dependency_scanning: %i[dependency_scanning],
container_scanning: %i[container_scanning sast_container], container_scanning: %i[container_scanning sast_container],
dast: %i[dast] dast: %i[dast],
performance: %i[:merge_request_performance_metrics]
}.freeze }.freeze
# Deprecated, to be removed in 12.0 # Deprecated, to be removed in 12.0
...@@ -54,6 +55,10 @@ module EE ...@@ -54,6 +55,10 @@ module EE
dast: { dast: {
names: %w(dast), names: %w(dast),
files: %w(gl-dast-report.json) files: %w(gl-dast-report.json)
},
performance: {
names: %w(performance deploy),
files: %w(performance.json)
} }
}.freeze }.freeze
...@@ -96,10 +101,6 @@ module EE ...@@ -96,10 +101,6 @@ module EE
nil nil
end end
def performance_artifact
@performance_artifact ||= artifacts_with_files.find(&:has_performance_json?)
end
def license_management_artifact def license_management_artifact
@license_management_artifact ||= artifacts_with_files.find(&:has_license_management_json?) @license_management_artifact ||= artifacts_with_files.find(&:has_license_management_json?)
end end
...@@ -108,20 +109,11 @@ module EE ...@@ -108,20 +109,11 @@ module EE
license_management_artifact&.success? license_management_artifact&.success?
end end
def has_performance_data?
performance_artifact&.success?
end
def expose_license_management_data? def expose_license_management_data?
project.feature_available?(:license_management) && project.feature_available?(:license_management) &&
has_license_management_data? has_license_management_data?
end end
def expose_performance_data?
project.feature_available?(:merge_request_performance_metrics) &&
has_performance_data?
end
def has_security_reports? def has_security_reports?
complete? && builds.latest.with_security_reports.any? complete? && builds.latest.with_security_reports.any?
end end
......
...@@ -16,8 +16,6 @@ module EE ...@@ -16,8 +16,6 @@ module EE
validate :validate_approvals_before_merge, unless: :importing? validate :validate_approvals_before_merge, unless: :importing?
delegate :performance_artifact, to: :head_pipeline, prefix: :head, allow_nil: true
delegate :performance_artifact, to: :base_pipeline, prefix: :base, allow_nil: true
delegate :license_management_artifact, to: :head_pipeline, prefix: :head, allow_nil: true delegate :license_management_artifact, to: :head_pipeline, prefix: :head, allow_nil: true
delegate :license_management_artifact, to: :base_pipeline, prefix: :base, allow_nil: true delegate :license_management_artifact, to: :base_pipeline, prefix: :base, allow_nil: true
delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true
...@@ -40,11 +38,6 @@ module EE ...@@ -40,11 +38,6 @@ module EE
false false
end end
def expose_performance_data?
!!(head_pipeline&.expose_performance_data? &&
base_pipeline&.expose_performance_data?)
end
def validate_approvals_before_merge def validate_approvals_before_merge
return true unless approvals_before_merge return true unless approvals_before_merge
return true unless target_project return true unless target_project
......
...@@ -26,17 +26,13 @@ module EE ...@@ -26,17 +26,13 @@ module EE
end end
end end
expose :performance, if: -> (mr, _) { mr.expose_performance_data? } do expose :performance, if: -> (mr, _) { head_pipeline_downloadable_path_for_report_type(:performance) } do
expose :head_path, if: -> (mr, _) { can?(current_user, :read_build, mr.head_performance_artifact) } do |merge_request| expose :head_path do |merge_request|
raw_project_build_artifacts_url(merge_request.source_project, head_pipeline_downloadable_path_for_report_type(:performance)
merge_request.head_performance_artifact,
path: Ci::Build::PERFORMANCE_FILE)
end end
expose :base_path, if: -> (mr, _) { can?(current_user, :read_build, mr.base_performance_artifact) } do |merge_request| expose :base_path do |merge_request|
raw_project_build_artifacts_url(merge_request.target_project, base_pipeline_downloadable_path_for_report_type(:performance)
merge_request.base_performance_artifact,
path: Ci::Build::PERFORMANCE_FILE)
end end
end end
......
...@@ -116,10 +116,6 @@ describe Ci::Build do ...@@ -116,10 +116,6 @@ describe Ci::Build do
end end
build_artifacts_methods = { build_artifacts_methods = {
has_performance_json?: {
filename: Ci::Build::PERFORMANCE_FILE,
job_names: %w[performance deploy]
},
has_license_management_json?: { has_license_management_json?: {
filename: Ci::Build::LICENSE_MANAGEMENT_FILE, filename: Ci::Build::LICENSE_MANAGEMENT_FILE,
job_names: %w[license_management] job_names: %w[license_management]
......
...@@ -21,7 +21,6 @@ describe Ci::Pipeline do ...@@ -21,7 +21,6 @@ describe Ci::Pipeline do
end end
PIPELINE_ARTIFACTS_METHODS = [ PIPELINE_ARTIFACTS_METHODS = [
{ method: :performance_artifact, options: [Ci::Build::PERFORMANCE_FILE, 'performance'] },
{ method: :license_management_artifact, options: [Ci::Build::LICENSE_MANAGEMENT_FILE, 'license_management'] } { method: :license_management_artifact, options: [Ci::Build::LICENSE_MANAGEMENT_FILE, 'license_management'] }
].freeze ].freeze
...@@ -277,31 +276,6 @@ describe Ci::Pipeline do ...@@ -277,31 +276,6 @@ describe Ci::Pipeline do
end end
end end
context 'performance' do
def create_build(job_name, filename)
create(
:ci_build,
:artifacts,
name: job_name,
pipeline: pipeline,
options: {
artifacts: {
paths: [filename]
}
}
)
end
it 'does not perform extra queries when calling pipeline artifacts methods after the first' do
create_build('performance', 'performance.json')
create_build('license_management', 'gl-license-management-report.json')
pipeline.performance_artifact
expect { pipeline.license_management_artifact }.not_to exceed_query_limit(0)
end
end
describe '#has_security_reports?' do describe '#has_security_reports?' do
subject { pipeline.has_security_reports? } subject { pipeline.has_security_reports? }
......
...@@ -41,28 +41,6 @@ describe MergeRequest do ...@@ -41,28 +41,6 @@ describe MergeRequest do
it { expect(subject.base_pipeline).to eq(pipeline) } it { expect(subject.base_pipeline).to eq(pipeline) }
end end
describe '#base_performance_artifact' do
before do
allow(subject.base_pipeline).to receive(:performance_artifact)
.and_return(1)
end
it 'delegates to merge request diff' do
expect(subject.base_performance_artifact).to eq(1)
end
end
describe '#head_performance_artifact' do
before do
allow(subject.head_pipeline).to receive(:performance_artifact)
.and_return(1)
end
it 'delegates to merge request diff' do
expect(subject.head_performance_artifact).to eq(1)
end
end
describe '#base_license_management_artifact' do describe '#base_license_management_artifact' do
before do before do
allow(subject.base_pipeline).to receive(:license_management_artifact) allow(subject.base_pipeline).to receive(:license_management_artifact)
...@@ -85,23 +63,6 @@ describe MergeRequest do ...@@ -85,23 +63,6 @@ describe MergeRequest do
end end
end end
describe '#expose_performance_data?' do
context 'with performance data' do
let(:pipeline) { double(expose_performance_data?: true) }
before do
allow(subject).to receive(:head_pipeline).and_return(pipeline)
allow(subject).to receive(:base_pipeline).and_return(pipeline)
end
it { expect(subject.expose_performance_data?).to be_truthy }
end
context 'without performance data' do
it { expect(subject.expose_performance_data?).to be_falsey }
end
end
describe '#expose_license_management_data?' do describe '#expose_license_management_data?' do
before do before do
allow(subject.head_pipeline).to receive(:expose_license_management_data?) allow(subject.head_pipeline).to receive(:expose_license_management_data?)
......
...@@ -74,18 +74,6 @@ describe MergeRequestWidgetEntity do ...@@ -74,18 +74,6 @@ describe MergeRequestWidgetEntity do
end end
end end
it 'has performance data' do
build = create(:ci_build, name: 'job')
allow(merge_request).to receive_messages(
expose_performance_data?: true,
base_performance_artifact: build,
head_performance_artifact: build
)
expect(subject.as_json).to include(:performance)
end
describe '#license_management' do describe '#license_management' do
before do before do
build = create(:ci_build, name: 'license_management', pipeline: pipeline) build = create(:ci_build, name: 'license_management', pipeline: pipeline)
......
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