Merge branch 'detect-previous-tests-fork-project-pipelines' into 'master'

Make previous test detection work for other projects and hosts

See merge request gitlab-org/gitlab!73160

.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

scripts/pipeline_test_report_builder.rb

@@ -20,7 +20,7 @@ require_relative 'api/default_options'
 # 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 @@ class PipelineTestReportBuilder
     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 @@ class PipelineTestReportBuilder
 
     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 @@ class PipelineTestReportBuilder
       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 @@ if $0 == __FILE__
   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|

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...'
 

spec/scripts/pipeline_test_report_builder_spec.rb

@@ -9,30 +9,39 @@ RSpec.describe PipelineTestReportBuilder do
 
   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 @@ RSpec.describe PipelineTestReportBuilder do
   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