Commit bedd5200 authored by Can Eldem's avatar Can Eldem Committed by Rémy Coutable

Refactor error handling for report comparison

Update test for failure case
Add protection for nil pipeline
parent be156e6f
...@@ -24,24 +24,22 @@ module Security ...@@ -24,24 +24,22 @@ module Security
end end
def execute def execute
pipeline_reports.each_with_object([]) do |(type, report), occurrences| pipeline_reports&.each_with_object([]) do |(type, report), occurrences|
next unless requested_type?(type) next unless requested_type?(type)
raise ParseError, 'JSON parsing failed' if report.error.is_a?(Gitlab::Ci::Parsers::Security::Common::SecurityReportParserError)
normalized_occurrences = normalize_report_occurrences(report.occurrences) normalized_occurrences = normalize_report_occurrences(report.occurrences)
filtered_occurrences = filter(normalized_occurrences) filtered_occurrences = filter(normalized_occurrences)
occurrences.concat(filtered_occurrences) occurrences.concat(filtered_occurrences)
end end
# Created follow-up issue to better handle exception case - https://gitlab.com/gitlab-org/gitlab/issues/14007
rescue NoMethodError => _ # propagate error for CompareReportsBaseService
raise ParseError, 'JSON parsing failed'
end end
private private
def pipeline_reports def pipeline_reports
pipeline.security_reports.reports pipeline&.security_reports&.reports
end end
def normalize_report_occurrences(report_occurrences) def normalize_report_occurrences(report_occurrences)
......
...@@ -59,13 +59,20 @@ describe Ci::CompareContainerScanningReportsService do ...@@ -59,13 +59,20 @@ describe Ci::CompareContainerScanningReportsService do
end end
context 'when head pipeline has corrupted container scanning vulnerability reports' do context 'when head pipeline has corrupted container scanning vulnerability reports' do
let!(:base_pipeline) { nil } let!(:base_pipeline) { create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) }
let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) } let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) }
it 'returns status and error message' do it 'returns status and error message' do
expect(subject[:status]).to eq(:error) expect(subject[:status]).to eq(:error)
expect(subject[:status_reason]).to include('JSON parsing failed') expect(subject[:status_reason]).to include('JSON parsing failed')
end end
it 'returns status and error message when pipeline is nil' do
result = service.execute(nil, head_pipeline)
expect(result[:status]).to eq(:error)
expect(result[:status_reason]).to include('JSON parsing failed')
end
end end
end end
end end
...@@ -61,13 +61,20 @@ describe Ci::CompareDependencyScanningReportsService do ...@@ -61,13 +61,20 @@ describe Ci::CompareDependencyScanningReportsService do
end end
context 'when head pipeline has corrupted dependency scanning vulnerability reports' do context 'when head pipeline has corrupted dependency scanning vulnerability reports' do
let!(:base_pipeline) { nil } let!(:base_pipeline) { create(:ee_ci_pipeline, :with_corrupted_dependency_scanning_report, project: project) }
let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_dependency_scanning_report, project: project) } let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_dependency_scanning_report, project: project) }
it 'returns status and error message' do it 'returns status and error message' do
expect(subject[:status]).to eq(:error) expect(subject[:status]).to eq(:error)
expect(subject[:status_reason]).to include('JSON parsing failed') expect(subject[:status_reason]).to include('JSON parsing failed')
end end
it 'returns status and error message when pipeline is nil' do
result = service.execute(nil, head_pipeline)
expect(result[:status]).to eq(:error)
expect(result[:status_reason]).to include('JSON parsing failed')
end
end end
end end
end end
...@@ -50,13 +50,20 @@ describe Ci::CompareLicenseManagementReportsService do ...@@ -50,13 +50,20 @@ describe Ci::CompareLicenseManagementReportsService do
end end
context 'when head pipeline has corrupted license management reports' do context 'when head pipeline has corrupted license management reports' do
let!(:base_pipeline) { nil } let!(:base_pipeline) { create(:ee_ci_pipeline, :with_corrupted_license_management_report, project: project) }
let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_license_management_report, project: project) } let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_license_management_report, project: project) }
it 'returns status and error message' do it 'returns status and error message' do
expect(subject[:status]).to eq(:error) expect(subject[:status]).to eq(:error)
expect(subject[:status_reason]).to include('JSON parsing failed') expect(subject[:status_reason]).to include('JSON parsing failed')
end end
it 'returns status and error message when pipeline is nil' do
result = service.execute(nil, head_pipeline)
expect(result[:status]).to eq(:error)
expect(result[:status_reason]).to include('JSON parsing failed')
end
end end
end end
end end
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