From 87f9cb8c0e278253af9b49a939c67a5dca732abf Mon Sep 17 00:00:00 2001
From: Mark Fletcher <mark@gitlab.com>
Date: Wed, 3 Nov 2021 20:03:39 +0000
Subject: [PATCH] Make previous test detection work for other projects and
hosts
---
.gitlab/ci/rules.gitlab-ci.yml | 4 -
scripts/pipeline_test_report_builder.rb | 42 ++---
scripts/rspec_helpers.sh | 7 +-
.../pipeline_test_report_builder_spec.rb | 174 +++++++++++-------
4 files changed, 137 insertions(+), 90 deletions(-)
diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml
index 8a782ec2b6844..fc68b8aa35475 100644
--- a/.gitlab/ci/rules.gitlab-ci.yml
+++ b/.gitlab/ci/rules.gitlab-ci.yml
@@ -1263,16 +1263,12 @@
.rails:rules:detect-previous-failed-tests:
rules:
- - <<: *if-not-canonical-namespace
- when: never
- <<: *if-merge-request-labels-run-all-rspec
- <<: *if-merge-request
changes: *code-backstage-patterns
.rails:rules:rerun-previous-failed-tests:
rules:
- - <<: *if-not-canonical-namespace
- when: never
- <<: *if-merge-request-labels-run-all-rspec
- <<: *if-merge-request
changes: *code-backstage-patterns
diff --git a/scripts/pipeline_test_report_builder.rb b/scripts/pipeline_test_report_builder.rb
index 56491d40a3ea2..2101decf59ac2 100755
--- a/scripts/pipeline_test_report_builder.rb
+++ b/scripts/pipeline_test_report_builder.rb
@@ -20,7 +20,7 @@
# Push into expected format for failed tests
class PipelineTestReportBuilder
def initialize(options)
- @project = options.delete(:project)
+ @target_project = options.delete(:target_project)
@mr_id = options.delete(:mr_id) || Host::DEFAULT_OPTIONS[:mr_id]
@instance_base_url = options.delete(:instance_base_url) || Host::DEFAULT_OPTIONS[:instance_base_url]
@output_file_path = options.delete(:output_file_path)
@@ -40,38 +40,38 @@ def execute
end
end
- private
+ def previous_pipeline
+ # Top of the list will always be the current pipeline
+ # Second from top will be the previous pipeline
+ pipelines_for_mr.sort_by { |a| -Time.parse(a['created_at']).to_i }[1]
+ end
- attr_reader :project, :mr_id, :instance_base_url, :output_file_path
+ private
- def project_api_base_url
- "#{instance_base_url}/api/v4/projects/#{CGI.escape(project)}"
- end
+ attr_reader :target_project, :mr_id, :instance_base_url, :output_file_path
- def project_base_url
- "#{instance_base_url}/#{project}"
+ def pipeline_project_api_base_url(pipeline)
+ "#{instance_base_url}/api/v4/projects/#{pipeline['project_id']}"
end
- def previous_pipeline
- # Top of the list will always be the current pipeline
- # Second from top will be the previous pipeline
- pipelines_for_mr.sort_by { |a| -Time.parse(a['created_at']).to_i }[1]
+ def target_project_api_base_url
+ "#{instance_base_url}/api/v4/projects/#{CGI.escape(target_project)}"
end
def pipelines_for_mr
- fetch("#{project_api_base_url}/merge_requests/#{mr_id}/pipelines")
+ fetch("#{target_project_api_base_url}/merge_requests/#{mr_id}/pipelines")
end
- def failed_builds_for_pipeline(pipeline_id)
- fetch("#{project_api_base_url}/pipelines/#{pipeline_id}/jobs?scope=failed&per_page=100")
+ def failed_builds_for_pipeline(pipeline)
+ fetch("#{pipeline_project_api_base_url(pipeline)}/pipelines/#{pipeline['id']}/jobs?scope=failed&per_page=100")
end
# Method uses the test suite endpoint to gather test results for a particular build.
# Here we request individual builds, even though it is possible to supply multiple build IDs.
# The reason for this; it is possible to lose the job context and name when requesting multiple builds.
# Please see for more info: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69053#note_709939709
- def test_report_for_build(pipeline_id, build_id)
- fetch("#{project_base_url}/-/pipelines/#{pipeline_id}/tests/suite.json?build_ids[]=#{build_id}")
+ def test_report_for_build(pipeline, build_id)
+ fetch("#{pipeline['web_url']}/tests/suite.json?build_ids[]=#{build_id}")
end
def build_test_report_json_for_pipeline(pipeline)
@@ -82,7 +82,7 @@ def build_test_report_json_for_pipeline(pipeline)
puts "Discovered last failed pipeline (#{pipeline['id']}) for MR!#{mr_id}"
- failed_builds_for_test_stage = failed_builds_for_pipeline(pipeline['id']).select do |failed_build|
+ failed_builds_for_test_stage = failed_builds_for_pipeline(pipeline).select do |failed_build|
failed_build['stage'] == 'test'
end
@@ -92,7 +92,7 @@ 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['id'], failed_build['id'])
+ test_report['suites'] << test_report_for_build(pipeline, failed_build['id'])
end
end
@@ -127,8 +127,8 @@ def fetch(uri_str)
options = Host::DEFAULT_OPTIONS.dup
OptionParser.new do |opts|
- opts.on("-p", "--project PROJECT", String, "Project where to find the merge request(defaults to $CI_PROJECT_ID)") do |value|
- options[:project] = value
+ opts.on("-t", "--target-project TARGET_PROJECT", String, "Project where to find the merge request") do |value|
+ options[:target_project] = value
end
opts.on("-m", "--mr-id MR_ID", String, "A merge request ID") do |value|
diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh
index 7ac6b98938fd0..a3565db58fabf 100644
--- a/scripts/rspec_helpers.sh
+++ b/scripts/rspec_helpers.sh
@@ -94,11 +94,14 @@ function retrieve_previous_failed_tests() {
local rspec_pg_regex="${2}"
local rspec_ee_pg_regex="${3}"
local pipeline_report_path="test_results/previous/test_reports.json"
- local project_path="gitlab-org/gitlab"
+
+ # Used to query merge requests. This variable reflects where the merge request has been created
+ local target_project_path="${CI_MERGE_REQUEST_PROJECT_PATH}"
+ local instance_url="${CI_SERVER_URL}"
echo 'Attempting to build pipeline test report...'
- scripts/pipeline_test_report_builder.rb --instance-base-url "https://gitlab.com" --project "${project_path}" --mr-id "${CI_MERGE_REQUEST_IID}" --output-file-path "${pipeline_report_path}"
+ scripts/pipeline_test_report_builder.rb --instance-base-url "${instance_url}" --target-project "${target_project_path}" --mr-id "${CI_MERGE_REQUEST_IID}" --output-file-path "${pipeline_report_path}"
echo 'Generating failed tests lists...'
diff --git a/spec/scripts/pipeline_test_report_builder_spec.rb b/spec/scripts/pipeline_test_report_builder_spec.rb
index 8a5388f4db8a0..8553ada044ef1 100644
--- a/spec/scripts/pipeline_test_report_builder_spec.rb
+++ b/spec/scripts/pipeline_test_report_builder_spec.rb
@@ -9,30 +9,39 @@
subject do
described_class.new(
- project: 'gitlab-org/gitlab',
+ target_project: 'gitlab-org/gitlab',
mr_id: '999',
instance_base_url: 'https://gitlab.com',
output_file_path: output_file_path
)
end
- let(:mr_pipelines) do
- [
- {
- 'status' => 'running',
- 'created_at' => DateTime.now.to_s
- },
- {
- 'status' => 'failed',
- 'created_at' => (DateTime.now - 5).to_s
- }
- ]
+ let(:failed_pipeline_url) { 'pipeline2_url' }
+
+ let(:failed_pipeline) do
+ {
+ 'status' => 'failed',
+ 'created_at' => (DateTime.now - 5).to_s,
+ 'web_url' => failed_pipeline_url
+ }
end
+ let(:current_pipeline) do
+ {
+ 'status' => 'running',
+ 'created_at' => DateTime.now.to_s,
+ 'web_url' => 'pipeline1_url'
+ }
+ end
+
+ let(:mr_pipelines) { [current_pipeline, failed_pipeline] }
+
+ let(:failed_build_id) { 9999 }
+
let(:failed_builds_for_pipeline) do
[
{
- 'id' => 9999,
+ 'id' => failed_build_id,
'stage' => 'test'
}
]
@@ -62,75 +71,114 @@
before do
allow(subject).to receive(:pipelines_for_mr).and_return(mr_pipelines)
allow(subject).to receive(:failed_builds_for_pipeline).and_return(failed_builds_for_pipeline)
- allow(subject).to receive(:test_report_for_build).and_return(test_report_for_build)
end
- describe '#test_report_for_latest_pipeline' do
- context 'no previous pipeline' do
- let(:mr_pipelines) { [] }
+ describe '#previous_pipeline' do
+ let(:fork_pipeline_url) { 'fork_pipeline_url' }
+ let(:fork_pipeline) do
+ {
+ 'status' => 'failed',
+ 'created_at' => (DateTime.now - 5).to_s,
+ 'web_url' => fork_pipeline_url
+ }
+ end
- it 'returns empty hash' do
- expect(subject.test_report_for_latest_pipeline).to eq("{}")
- end
+ before do
+ allow(subject).to receive(:test_report_for_build).and_return(test_report_for_build)
end
- context 'first pipeline scenario' do
- let(:mr_pipelines) do
- [
- {
- 'status' => 'running',
- 'created_at' => DateTime.now.to_s
- }
- ]
+ context 'pipeline in a fork project' do
+ let(:mr_pipelines) { [current_pipeline, fork_pipeline] }
+
+ it 'returns fork pipeline' do
+ expect(subject.previous_pipeline).to eq(fork_pipeline)
end
+ end
- it 'returns empty hash' do
- expect(subject.test_report_for_latest_pipeline).to eq("{}")
+ context 'pipeline in target project' do
+ it 'returns failed pipeline' do
+ expect(subject.previous_pipeline).to eq(failed_pipeline)
end
end
+ end
- context 'no previous failed pipeline' do
- let(:mr_pipelines) do
- [
- {
- 'status' => 'running',
- 'created_at' => DateTime.now.to_s
- },
- {
- 'status' => 'success',
- 'created_at' => (DateTime.now - 5).to_s
- }
- ]
+ describe '#test_report_for_latest_pipeline' do
+ 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
+ end
+
+ context 'canonical pipeline' do
+ before do
+ allow(subject).to receive(:test_report_for_build).and_return(test_report_for_build)
end
- it 'returns empty hash' do
- expect(subject.test_report_for_latest_pipeline).to eq("{}")
+ context 'no previous pipeline' do
+ let(:mr_pipelines) { [] }
+
+ it 'returns empty hash' do
+ expect(subject.test_report_for_latest_pipeline).to eq("{}")
+ end
end
- end
- context 'no failed test builds' do
- let(:failed_builds_for_pipeline) do
- [
- {
- 'id' => 9999,
- 'stage' => 'prepare'
- }
- ]
+ context 'first pipeline scenario' do
+ let(:mr_pipelines) do
+ [
+ {
+ 'status' => 'running',
+ 'created_at' => DateTime.now.to_s
+ }
+ ]
+ end
+
+ it 'returns empty hash' do
+ expect(subject.test_report_for_latest_pipeline).to eq("{}")
+ end
end
- it 'returns empty hash' do
- expect(subject.test_report_for_latest_pipeline).to eq("{}")
+ context 'no previous failed pipeline' do
+ let(:mr_pipelines) do
+ [
+ {
+ 'status' => 'running',
+ 'created_at' => DateTime.now.to_s
+ },
+ {
+ 'status' => 'success',
+ 'created_at' => (DateTime.now - 5).to_s
+ }
+ ]
+ end
+
+ it 'returns empty hash' do
+ expect(subject.test_report_for_latest_pipeline).to eq("{}")
+ end
+ end
+
+ context 'no failed test builds' do
+ let(:failed_builds_for_pipeline) do
+ [
+ {
+ 'id' => 9999,
+ 'stage' => 'prepare'
+ }
+ ]
+ end
+
+ it 'returns empty hash' do
+ expect(subject.test_report_for_latest_pipeline).to eq("{}")
+ end
end
- end
- context 'failed pipeline and failed test builds' do
- it 'returns populated test list for suites' do
- actual = subject.test_report_for_latest_pipeline
- expected = {
- 'suites' => [test_report_for_build]
- }.to_json
+ context 'failed pipeline and failed test builds' do
+ it 'returns populated test list for suites' do
+ actual = subject.test_report_for_latest_pipeline
+ expected = {
+ 'suites' => [test_report_for_build]
+ }.to_json
- expect(actual).to eq(expected)
+ expect(actual).to eq(expected)
+ end
end
end
end
--
GitLab