From 3eba2ad17fdff451363a88e67a0153331b178afc Mon Sep 17 00:00:00 2001
From: Brian Williams <bwilliams@gitlab.com>
Date: Wed, 6 Jul 2022 09:50:58 +0000
Subject: [PATCH] Allow PipelineTestReportBuilder to handle missing test
reports
---
scripts/pipeline_test_report_builder.rb | 7 ++-
.../pipeline_test_report_builder_spec.rb | 48 ++++++++++++++++---
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/scripts/pipeline_test_report_builder.rb b/scripts/pipeline_test_report_builder.rb
index 2101decf59ac2..5299dba3f9753 100755
--- a/scripts/pipeline_test_report_builder.rb
+++ b/scripts/pipeline_test_report_builder.rb
@@ -72,6 +72,10 @@ def failed_builds_for_pipeline(pipeline)
# Please see for more info: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69053#note_709939709
def test_report_for_build(pipeline, build_id)
fetch("#{pipeline['web_url']}/tests/suite.json?build_ids[]=#{build_id}")
+ rescue Net::HTTPServerException => e
+ raise e unless e.response.code == 404
+
+ puts "Artifacts not found. They may have expired. Skipping this build."
end
def build_test_report_json_for_pipeline(pipeline)
@@ -92,7 +96,8 @@ def build_test_report_json_for_pipeline(pipeline)
test_report['suites'] ||= []
failed_builds_for_test_stage.each do |failed_build|
- test_report['suites'] << test_report_for_build(pipeline, failed_build['id'])
+ suite = test_report_for_build(pipeline, failed_build['id'])
+ test_report['suites'] << suite if suite
end
end
diff --git a/spec/scripts/pipeline_test_report_builder_spec.rb b/spec/scripts/pipeline_test_report_builder_spec.rb
index 8553ada044ef1..198cdefc53008 100644
--- a/spec/scripts/pipeline_test_report_builder_spec.rb
+++ b/spec/scripts/pipeline_test_report_builder_spec.rb
@@ -103,16 +103,18 @@
end
describe '#test_report_for_latest_pipeline' do
+ let(:failed_build_uri) { "#{failed_pipeline_url}/tests/suite.json?build_ids[]=#{failed_build_id}" }
+
+ before do
+ allow(subject).to receive(:fetch).with(failed_build_uri).and_return(failed_builds_for_pipeline)
+ end
+
it 'fetches builds from pipeline related to MR' do
- expect(subject).to receive(:fetch).with("#{failed_pipeline_url}/tests/suite.json?build_ids[]=#{failed_build_id}").and_return(failed_builds_for_pipeline)
- subject.test_report_for_latest_pipeline
+ expected = { "suites" => [failed_builds_for_pipeline] }.to_json
+ expect(subject.test_report_for_latest_pipeline).to eq(expected)
end
context 'canonical pipeline' do
- before do
- allow(subject).to receive(:test_report_for_build).and_return(test_report_for_build)
- end
-
context 'no previous pipeline' do
let(:mr_pipelines) { [] }
@@ -171,6 +173,10 @@
end
context 'failed pipeline and failed test builds' do
+ before do
+ allow(subject).to receive(:fetch).with(failed_build_uri).and_return(test_report_for_build)
+ end
+
it 'returns populated test list for suites' do
actual = subject.test_report_for_latest_pipeline
expected = {
@@ -180,6 +186,36 @@
expect(actual).to eq(expected)
end
end
+
+ context 'when receiving a server error' do
+ let(:response) { instance_double('Net::HTTPResponse') }
+ let(:error) { Net::HTTPServerException.new('server error', response) }
+ let(:test_report_for_latest_pipeline) { subject.test_report_for_latest_pipeline }
+
+ before do
+ allow(response).to receive(:code).and_return(response_code)
+ allow(subject).to receive(:fetch).with(failed_build_uri).and_raise(error)
+ end
+
+ context 'when response code is 404' do
+ let(:response_code) { 404 }
+
+ it 'continues without the missing reports' do
+ expected = { 'suites' => [] }.to_json
+
+ expect { test_report_for_latest_pipeline }.not_to raise_error
+ expect(test_report_for_latest_pipeline).to eq(expected)
+ end
+ end
+
+ context 'when response code is unexpected' do
+ let(:response_code) { 500 }
+
+ it 'raises HTTPServerException' do
+ expect { test_report_for_latest_pipeline }.to raise_error(error)
+ end
+ end
+ end
end
end
end
--
GitLab